diff --git a/git_cl.py b/git_cl.py index aae111e0e6..d8db3ade0b 100755 --- a/git_cl.py +++ b/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 diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 22269b534f..8b9b9d4cae 100755 --- a/tests/git_cl_test.py +++ b/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):