Move code starting the editor into a common function.

Windows users wouldn't get the same behavior on git cl vs gcl.
Improve automatic CRLF<->LF conversion, some gcl users would be \n repeated in
their description depending on the editor used.

[email protected]
BUG=
TEST=


Review URL: http://codereview.chromium.org/8360007

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@107106 0039d316-1c4b-4281-b951-d872f2087c98

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)
 

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

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)

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:

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',

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)