diff --git a/drover.py b/drover.py index f473955d2..ef8cee50b 100755 --- a/drover.py +++ b/drover.py @@ -477,7 +477,10 @@ def drover(options, args): change_cmd = 'change ' + str(revision) + " " + filename if options.revertbot: - change_cmd += ' --silent' + if sys.platform == 'win32': + os.environ['SVN_EDITOR'] = 'cmd.exe /c exit' + else: + os.environ['SVN_EDITOR'] = 'true' runGcl(change_cmd) os.unlink(filename) diff --git a/gcl.py b/gcl.py index 8df5de1bc..a63af71d8 100755 --- a/gcl.py +++ b/gcl.py @@ -733,20 +733,6 @@ def ListFiles(show_unknown_files): return 0 -def GetEditor(): - editor = os.environ.get("SVN_EDITOR") - if not editor: - editor = os.environ.get("EDITOR") - - if not editor: - if sys.platform.startswith("win"): - editor = "notepad" - else: - editor = "vi" - - return editor - - def GenerateDiff(files, root=None): return SVN.GenerateDiff(files, root=root) @@ -1054,7 +1040,6 @@ def CMDchange(args): else: changename = args[0] change_info = ChangeInfo.Load(changename, GetRepositoryRoot(), False, True) - silent = FilterFlag(args, "--silent") # Verify the user is running the change command from a read-write checkout. svn_info = SVN.CaptureInfo('.') @@ -1103,11 +1088,7 @@ def CMDchange(args): "---Paths in this changelist (" + change_info.name + "):\n") separator2 = "\n\n---Paths modified but not in any changelist:\n\n" - description_to_write = description - if sys.platform == 'win32': - description_to_write = description.replace('\n', '\r\n') - - text = (description_to_write + separator1 + '\n' + + text = (description + separator1 + '\n' + '\n'.join([f[0] + f[1] for f in change_info.GetFiles()])) if change_info.Exists(): @@ -1118,40 +1099,16 @@ def CMDchange(args): separator2) text += '\n'.join([f[0] + f[1] for f in unaffected_files]) + '\n' - handle, filename = tempfile.mkstemp(text=True) - os.write(handle, text) - os.close(handle) - - # Open up the default editor in the system to get the CL description. - try: - if not silent: - cmd = '%s %s' % (GetEditor(), filename) - if sys.platform == 'win32' and os.environ.get('TERM') == 'msys': - # Msysgit requires the usage of 'env' to be present. - cmd = 'env ' + cmd - try: - # shell=True to allow the shell to handle all forms of quotes in - # $EDITOR. - subprocess2.check_call(cmd, shell=True) - except subprocess2.CalledProcessError, e: - ErrorExit('Editor returned %d' % e.returncode) - result = gclient_utils.FileRead(filename) - finally: - os.remove(filename) - + result = gclient_utils.RunEditor(text, False) if not result: - return 0 + ErrorExit('Running editor failed') split_result = result.split(separator1, 1) if len(split_result) != 2: - ErrorExit("Don't modify the text starting with ---!\n\n" + result) + ErrorExit("Don't modify the text starting with ---!\n\n%r" % result) # Update the CL description if it has changed. new_description = split_result[0] - - if sys.platform == 'win32': - new_description = new_description.replace('\r\n', '\n') - cl_files_text = split_result[1] if new_description != description or override_description: change_info.description = new_description diff --git a/gclient_utils.py b/gclient_utils.py index 1981f36e7..8709e0819 100644 --- a/gclient_utils.py +++ b/gclient_utils.py @@ -11,6 +11,7 @@ import re import stat import sys +import tempfile import threading import time @@ -687,3 +688,46 @@ def run(self): work_queue.ready_cond.notifyAll() finally: work_queue.ready_cond.release() + + +def GetEditor(git): + """Returns the most plausible editor to use.""" + if git: + editor = os.environ.get('GIT_EDITOR') + else: + editor = os.environ.get('SVN_EDITOR') + if not editor: + editor = os.environ.get('EDITOR') + if not editor: + if sys.platform.startswith('win'): + editor = 'notepad' + else: + editor = 'vim' + return editor + + +def RunEditor(content, git): + """Opens up the default editor in the system to get the CL description.""" + file_handle, filename = tempfile.mkstemp(text=True) + # Make sure CRLF is handled properly by requiring none. + if '\r' in content: + print >> sys.stderr, ('!! Please remove \\r from your content !!') + fileobj = os.fdopen(file_handle, 'w') + # Still remove \r if present. + fileobj.write(re.sub('\r?\n', '\n', content)) + fileobj.close() + + try: + cmd = '%s %s' % (GetEditor(git), filename) + if sys.platform == 'win32' and os.environ.get('TERM') == 'msys': + # Msysgit requires the usage of 'env' to be present. + cmd = 'env ' + cmd + try: + # shell=True to allow the shell to handle all forms of quotes in + # $EDITOR. + subprocess2.check_call(cmd, shell=True) + except subprocess2.CalledProcessError: + return None + return FileRead(filename) + finally: + os.remove(filename) diff --git a/git_cl.py b/git_cl.py index 9566118c5..1300267a7 100755 --- a/git_cl.py +++ b/git_cl.py @@ -12,7 +12,6 @@ import os import re import sys -import tempfile import textwrap import urlparse import urllib2 @@ -36,6 +35,7 @@ from third_party import upload import breakpad # pylint: disable=W0611 import fix_encoding +import gclient_utils import presubmit_support import rietveld import scm @@ -648,7 +648,13 @@ def Update(self): initial_text += '\nBUG=' if 'TEST=' not in self.description: initial_text += '\nTEST=' - self._ParseDescription(UserEditedLog(initial_text)) + content = gclient_utils.RunEditor(initial_text, True) + if not content: + DieWithError('Running editor failed') + content = re.compile(r'^#.*$', re.MULTILINE).sub('', content).strip() + if not content: + DieWithError('No CL description, aborting') + self._ParseDescription(content) def _ParseDescription(self, description): if not description: @@ -824,39 +830,6 @@ def CreateDescriptionFromLog(args): return RunGit(['log', '--pretty=format:%s\n\n%b'] + log_args) -def UserEditedLog(starting_text): - """Given some starting text, let the user edit it and return the result.""" - editor = os.getenv('EDITOR', 'vi') - - (file_handle, filename) = tempfile.mkstemp() - fileobj = os.fdopen(file_handle, 'w') - fileobj.write(starting_text) - fileobj.close() - - # Open up the default editor in the system to get the CL description. - try: - cmd = '%s %s' % (editor, filename) - if sys.platform == 'win32' and os.environ.get('TERM') == 'msys': - # Msysgit requires the usage of 'env' to be present. - cmd = 'env ' + cmd - # shell=True to allow the shell to handle all forms of quotes in $EDITOR. - try: - subprocess2.check_call(cmd, shell=True) - except subprocess2.CalledProcessError, e: - DieWithError('Editor returned %d' % e.returncode) - fileobj = open(filename) - text = fileobj.read() - fileobj.close() - finally: - os.remove(filename) - - if not text: - return - - stripcomment_re = re.compile(r'^#.*$', re.MULTILINE) - return stripcomment_re.sub('', text).strip() - - def ConvertToInteger(inputval): """Convert a string to integer, but returns either an int or None.""" try: diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index e013df2e2..41b40836d 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -84,7 +84,7 @@ def testMembersChanged(self): 'ErrorExit', 'FILES_CACHE', 'FilterFlag', 'GenUsage', 'GenerateChangeName', 'GenerateDiff', 'GetCLs', 'GetCacheDir', 'GetCachedFile', 'GetChangelistInfoFile', 'GetChangesDir', - 'GetCodeReviewSetting', 'GetEditor', 'GetFilesNotInCL', 'GetInfoDir', + 'GetCodeReviewSetting', 'GetFilesNotInCL', 'GetInfoDir', 'GetModifiedFiles', 'GetRepositoryRoot', 'ListFiles', 'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG', 'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT', 'REVIEWERS_REGEX', diff --git a/tests/gclient_utils_test.py b/tests/gclient_utils_test.py index ea4f50256..3e24c01b1 100755 --- a/tests/gclient_utils_test.py +++ b/tests/gclient_utils_test.py @@ -31,12 +31,13 @@ def testMembersChanged(self): 'Annotated', 'AutoFlush', 'CheckCallAndFilter', 'CheckCallAndFilterAndHeader', 'Error', 'ExecutionQueue', 'FileRead', 'FileWrite', 'FindFileUpwards', 'FindGclientRoot', - 'GetGClientRootAndEntries', 'IsDateRevision', 'MakeDateRevision', - 'MakeFileAutoFlush', 'MakeFileAnnotated', 'PathDifference', - 'PrintableObject', 'RemoveDirectory', 'SplitUrlRevision', - 'SyntaxErrorToError', 'Wrapper', 'WorkItem', + 'GetGClientRootAndEntries', 'GetEditor', 'IsDateRevision', + 'MakeDateRevision', 'MakeFileAutoFlush', 'MakeFileAnnotated', + 'PathDifference', 'PrintableObject', 'RemoveDirectory', 'RunEditor', + 'SplitUrlRevision', 'SyntaxErrorToError', 'Wrapper', 'WorkItem', 'errno', 'lockedmethod', 'logging', 'os', 'Queue', 're', 'rmtree', - 'safe_makedirs', 'stat', 'subprocess2', 'sys','threading', 'time', + 'safe_makedirs', 'stat', 'subprocess2', 'sys', 'tempfile', 'threading', + 'time', ] # If this test fails, you should add the relevant test. self.compareMembers(gclient_utils, members)