Run dartfmt when invoking git cl format.

git_cl.py

Index: git_cl.py
diff --git a/git_cl.py b/git_cl.py
index 1b375e856ca190fa4c7217cdfb30316f5c37a088..05114e753fa6afcb3e41a5b9bc92df3a0edfa2fe 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -36,6 +36,7 @@ from third_party import colorama
 from third_party import upload
 import breakpad  # pylint: disable=W0611
 import clang_format
+import dart_format
 import fix_encoding
 import gclient_utils
 import git_common
@@ -2893,6 +2894,24 @@ def CMDowners(parser, args):
       fopen=file, os_path=os.path, glob=glob.glob,
       disable_color=options.no_color).run()
 
+def BuildGitDiffCmd(diff_type, upstream_commit, args, extensions):
+  """Generates a diff command."""
+  # Generate diff for the current branch's changes.
+  diff_cmd = ['diff', '--no-ext-diff', '--no-prefix', diff_type,
+              upstream_commit, '--' ]
+
+  if args:
+    for arg in args:
+      if os.path.isdir(arg):
+        diff_cmd += [os.path.join(arg, '*' + ext) for ext in extensions]
+      elif os.path.isfile(arg):
+        diff_cmd.append(arg)
+      else:
+        DieWithError('Argument "%s" is not a file or a directory' % arg)
+  else:
+    diff_cmd += ['*' + ext for ext in extensions]
+
+  return diff_cmd
 
 @subcommand.usage('[files or directories to diff]')
 def CMDformat(parser, args):
@@ -2912,15 +2931,6 @@ def CMDformat(parser, args):
   if rel_base_path:
     os.chdir(rel_base_path)
 
-  # Generate diff for the current branch's changes.
-  diff_cmd = ['diff', '--no-ext-diff', '--no-prefix']
-  if opts.full:
-    # Only list the names of modified files.
-    diff_cmd.append('--name-only')
-  else:
-    # Only generate context-less patches.
-    diff_cmd.append('-U0')
-
   # Grab the merge-base commit, i.e. the upstream commit of the current
   # branch when it was created or the last time it was rebased. This is
   # to cover the case where the user may have called "git fetch origin",
@@ -2936,20 +2946,14 @@ def CMDformat(parser, args):
     DieWithError('Could not find base commit for this branch. '
                  'Are you in detached state?')
 
-  diff_cmd.append(upstream_commit)
-
-  # Handle source file filtering.
-  diff_cmd.append('--')
-  if args:
-    for arg in args:
-      if os.path.isdir(arg):
-        diff_cmd += [os.path.join(arg, '*' + ext) for ext in CLANG_EXTS]
-      elif os.path.isfile(arg):
-        diff_cmd.append(arg)
-      else:
-        DieWithError('Argument "%s" is not a file or a directory' % arg)
+  if opts.full:
+    # Only list the names of modified files.
+    clang_diff_type = '--name-only'
   else:
-    diff_cmd += ['*' + ext for ext in CLANG_EXTS]
+    # Only generate context-less patches.
+    clang_diff_type = '-U0'
+
+  diff_cmd = BuildGitDiffCmd(clang_diff_type, upstream_commit, args, CLANG_EXTS)
   diff_output = RunGit(diff_cmd)
 
   top_dir = os.path.normpath(
@@ -2964,15 +2968,13 @@ def CMDformat(parser, args):
   if opts.full:
     # diff_output is a list of files to send to clang-format.
     files = diff_output.splitlines()
-    if not files:
-      print "Nothing to format."
-      return 0
-    cmd = [clang_format_tool]
-    if not opts.dry_run and not opts.diff:
-      cmd.append('-i')
-    stdout = RunCommand(cmd + files, cwd=top_dir)
-    if opts.diff:
-      sys.stdout.write(stdout)
+    if files:
+      cmd = [clang_format_tool]
+      if not opts.dry_run and not opts.diff:
+        cmd.append('-i')
+      stdout = RunCommand(cmd + files, cwd=top_dir)
+      if opts.diff:
+        sys.stdout.write(stdout)
   else:
     env = os.environ.copy()
     env['PATH'] = str(os.path.dirname(clang_format_tool))
@@ -2990,9 +2992,32 @@ def CMDformat(parser, args):
     stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env)
     if opts.diff:
       sys.stdout.write(stdout)
+    # TODO(erg): Deal with this.
     if opts.dry_run and len(stdout) > 0:
       return 2

[Elliot Glaysher, 2015/02/18 20:41:42]

enne: you added the above two lines to return a value to the cmd prompt when a
dry run is done and there's output. What motivated this? Should I do something
analogous in the block below?

[enne (OOO), 2015/02/18 20:56:46]

Yeah, the "return 2" is to indicate to CheckPatchFormatted that "git cl format"
was not a no-op and that there were formatting changes.

See:
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/pres...

[Elliot Glaysher, 2015/02/18 21:16:17]

TYVM. I've modified this so that unformatted dart files cause this function to
return 2, and I've modified the mojo repository's presubmit scripts to run
CheckPatchFormatted, verifying that this works. (That will be landed
separately.)

 
+  # Build a diff command that only operates on dart files. dart's formatter
+  # does not have the nice property of only operating on modified chunks, so
+  # hard code full.
+  dart_diff_cmd = BuildGitDiffCmd('--name-only', upstream_commit,
+                                  args, ['.dart'])
+  dart_diff_output = RunGit(dart_diff_cmd)
+  if dart_diff_output:
+    try:
+      dartfmt = dart_format.FindDartFmtToolInChromiumTree()
+    except dart_format.NotFoundError, e:
+      DieWithError(e)
+
+    command = [dartfmt]
+    if not opts.dry_run and not opts.diff:
+      command.append('-w')
+    files = dart_diff_output.splitlines()
+    command.extend(files)
+
+    stdout = RunCommand(command, cwd=top_dir, env=env)
+    if opts.dry_run:
+      sys.stdout.write(stdout)
+
   return 0