Accept file list in `git cl diff`

A common pain point of mine is that when I rebase locally, `git cl diff`
starts showing many files unrelated to the CL. Accepting a file list
will allow me to filter for the files included in the CL. This is useful
in general, if a user is interested in the diff of a specific subset of
files.

Change-Id: Id7c328ab8cd201ecf855e45d29119672bec8d022
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/7768331
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Auto-Submit: Victor Vianna <victorvianna@google.com>

git_cl.py

@@ -6735,9 +6735,14 @@ def CMDset_close(parser, args):
     return 0
 
 
+@subcommand.usage('[--] [files ...]')
 @metrics.collector.collect_metrics('git cl diff')
-def CMDdiff(parser, args):
-    """Shows differences between local tree and last upload."""
+def CMDdiff(parser, raw_args):
+    """Shows differences between local tree and last upload.
+
+    positional arguments:
+      files           Files to diff. If omitted, diff all files.
+    """
     if gclient_utils.IsEnvCog():
         print(
             'diff command is not supported. Please navigate to source '
@@ -6749,9 +6754,7 @@ def CMDdiff(parser, args):
                       action='store_true',
                       dest='stat',
                       help='Generate a diffstat')
-    options, args = parser.parse_args(args)
-    if args:
-        parser.error('Unrecognized args: %s' % ' '.join(args))
+    options, args = parser.parse_args(raw_args)
 
     cl = Changelist()
     issue = cl.GetIssue()
@@ -6773,6 +6776,19 @@ def CMDdiff(parser, args):
     if options.stat:
         cmd.append('--stat')
     cmd.append(base)
+    # `git diff` behaves differently depending on whether the file list starts
+    # with '--' or not (using '--' won't check for file existence and so is
+    # useful to diff deleted files). The code below ensures `git cl diff`
+    # support both cases.
+    # OptionParser.parse() strips '--' so check raw_args and not args.
+    if '--' in raw_args:
+        if any(not a.startswith('-') for a in raw_args[:raw_args.index('--')]):
+            parser.error(
+                'All positional arguments must come after "--" when it is used.'
+            )
+        cmd.append('--')
+    if args:
+        cmd.extend(args)
     subprocess2.check_call(cmd)
 
     return 0

tests/git_cl_test.py

@@ -2834,6 +2834,41 @@ class TestGitCl(unittest.TestCase):
         self.assertEqual(0, git_cl.main(['description', '-n', '-']))
         self.assertEqual('hi\n\t there\n\nman', ChangelistMock.desc)
 
+    @unittest.skipIf(gclient_utils.IsEnvCog(),
+                     'not supported in non-git environment')
+    def test_diff_with_files(self):
+        mock.patch('git_common.current_branch', return_value='main').start()
+
+        scm.GIT.SetConfig('', 'branch.main.gerritissue', '123')
+        scm.GIT.SetConfig('', 'branch.main.last-upload-hash', 'deadbeaf')
+
+        self.calls = [
+            ((['git', 'diff', 'deadbeaf', 'file1'], ), ''),
+        ]
+        self.assertEqual(0, git_cl.main(['diff', 'file1']))
+
+        self.calls = [
+            ((['git', 'diff', 'deadbeaf', '--', 'file1'], ), ''),
+        ]
+        self.assertEqual(0, git_cl.main(['diff', '--', 'file1']))
+
+        with mock.patch('git_cl.OptionParser.error',
+                        side_effect=ParserErrorMock):
+            with self.assertRaises(ParserErrorMock):
+                git_cl.main(['diff', 'a.txt', '--', 'b.txt'])
+
+        self.calls = [
+            ((['git', 'diff', 'deadbeaf', '--', 'a.txt', 'b.txt'], ), ''),
+        ]
+        self.assertEqual(0, git_cl.main(['diff', '--', 'a.txt', 'b.txt']))
+
+        self.calls = [
+            ((['git', 'diff', '--stat', 'deadbeaf', '--', 'a.txt',
+               'b.txt'], ), ''),
+        ]
+        self.assertEqual(
+            0, git_cl.main(['diff', '--stat', '--', 'a.txt', 'b.txt']))
+
     @unittest.skipIf(gclient_utils.IsEnvCog(),
                     'not supported in non-git environment')
     def test_archive(self):