diff --git a/git_cl.py b/git_cl.py index 07fce1201..cf30eff8c 100755 --- a/git_cl.py +++ b/git_cl.py @@ -5117,56 +5117,6 @@ def BuildGitDiffCmd(diff_type, upstream_commit, args, allow_prefix=False): return diff_cmd -def _RunClangFormatDiff(opts, clang_diff_files, top_dir, upstream_commit): - """Runs clang-format-diff and sets a return value if necessary.""" - - if not clang_diff_files: - return 0 - - # Set to 2 to signal to CheckPatchFormatted() that this patch isn't - # formatted. This is used to block during the presubmit. - return_value = 0 - - # Locate the clang-format binary in the checkout - try: - clang_format_tool = clang_format.FindClangFormatToolInChromiumTree() - except clang_format.NotFoundError as e: - DieWithError(e) - - if opts.full or settings.GetFormatFullByDefault(): - cmd = [clang_format_tool] - if not opts.dry_run and not opts.diff: - cmd.append('-i') - stdout = RunCommand(cmd + clang_diff_files, cwd=top_dir) - if opts.diff: - sys.stdout.write(stdout) - if opts.dry_run and len(stdout) > 0: - return_value = 2 - else: - env = os.environ.copy() - env['PATH'] = str(os.path.dirname(clang_format_tool)) - try: - script = clang_format.FindClangFormatScriptInChromiumTree( - 'clang-format-diff.py') - except clang_format.NotFoundError as e: - DieWithError(e) - - cmd = [sys.executable, script, '-p0'] - if not opts.dry_run and not opts.diff: - cmd.append('-i') - - diff_cmd = BuildGitDiffCmd('-U0', upstream_commit, clang_diff_files) - diff_output = RunGit(diff_cmd) - - stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env) - if opts.diff: - sys.stdout.write(stdout) - if opts.dry_run and len(stdout) > 0: - return_value = 2 - - return return_value - - def MatchingFileType(file_name, extensions): """Returns True if the file name ends with one of the given extensions.""" return bool([ext for ext in extensions if file_name.lower().endswith(ext)]) @@ -5263,8 +5213,45 @@ def CMDformat(parser, args): top_dir = os.path.normpath( RunGit(["rev-parse", "--show-toplevel"]).rstrip('\n')) - return_value = _RunClangFormatDiff(opts, clang_diff_files, top_dir, - upstream_commit) + # Set to 2 to signal to CheckPatchFormatted() that this patch isn't + # formatted. This is used to block during the presubmit. + return_value = 0 + + if clang_diff_files: + # Locate the clang-format binary in the checkout + try: + clang_format_tool = clang_format.FindClangFormatToolInChromiumTree() + except clang_format.NotFoundError as e: + DieWithError(e) + + if opts.full or settings.GetFormatFullByDefault(): + cmd = [clang_format_tool] + if not opts.dry_run and not opts.diff: + cmd.append('-i') + stdout = RunCommand(cmd + clang_diff_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)) + try: + script = clang_format.FindClangFormatScriptInChromiumTree( + 'clang-format-diff.py') + except clang_format.NotFoundError as e: + DieWithError(e) + + cmd = [sys.executable, script, '-p0'] + if not opts.dry_run and not opts.diff: + cmd.append('-i') + + diff_cmd = BuildGitDiffCmd('-U0', upstream_commit, clang_diff_files) + diff_output = RunGit(diff_cmd) + + stdout = RunCommand(cmd, stdin=diff_output, cwd=top_dir, env=env) + if opts.diff: + sys.stdout.write(stdout) + if opts.dry_run and len(stdout) > 0: + return_value = 2 # Similar code to above, but using yapf on .py files rather than clang-format # on C/C++ files @@ -5349,7 +5336,7 @@ def CMDformat(parser, args): stdout = RunCommand(command, cwd=top_dir) if opts.dry_run and stdout: return_value = 2 - except dart_format.NotFoundError: + except dart_format.NotFoundError as e: print('Warning: Unable to check Dart code formatting. Dart SDK not ' 'found in this checkout. Files in other languages are still ' 'formatted.') diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index f4d20aca2..e3f6f57da 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -32,7 +32,6 @@ import metrics # We have to disable monitoring before importing git_cl. metrics.DISABLE_METRICS_COLLECTION = True -import clang_format import gerrit_util import git_cl import git_common @@ -3442,52 +3441,20 @@ class CMDFormatTestCase(TestCase): def setUp(self): super(CMDFormatTestCase, self).setUp() - mock.patch('git_cl.RunCommand').start() - mock.patch('clang_format.FindClangFormatToolInChromiumTree').start() - mock.patch('clang_format.FindClangFormatScriptInChromiumTree').start() - mock.patch('git_cl.settings').start() self._top_dir = tempfile.mkdtemp() - self.addCleanup(mock.patch.stopall) def tearDown(self): shutil.rmtree(self._top_dir) super(CMDFormatTestCase, self).tearDown() - def _make_temp_file(self, fname, contents): - with open(os.path.join(self._top_dir, fname), 'w') as tf: - tf.write('\n'.join(contents)) - def _make_yapfignore(self, contents): - self._make_temp_file('.yapfignore', contents) + with open(os.path.join(self._top_dir, '.yapfignore'), 'w') as yapfignore: + yapfignore.write('\n'.join(contents)) def _check_yapf_filtering(self, files, expected): self.assertEqual(expected, git_cl._FilterYapfIgnoredFiles( files, git_cl._GetYapfIgnorePatterns(self._top_dir))) - def testClangFormatDiff(self): - git_cl.RunCommand.return_value = 'error' - git_cl.settings.GetFormatFullByDefault.return_value = False - return_value = git_cl._RunClangFormatDiff( - mock.Mock(full=True, dry_run=True, diff=False), ['.'], self._top_dir, - 'HEAD') - self.assertEqual(2, return_value) - return_value = git_cl._RunClangFormatDiff( - mock.Mock(full=False, dry_run=True, diff=False), ['.'], self._top_dir, - 'HEAD') - self.assertEqual(2, return_value) - - def testClangFormatNoDiff(self): - git_cl.RunCommand.return_value = '' - git_cl.settings.GetFormatFullByDefault.return_value = False - return_value = git_cl._RunClangFormatDiff( - mock.Mock(full=True, dry_run=True, diff=False), ['.'], self._top_dir, - 'HEAD') - self.assertEqual(0, return_value) - return_value = git_cl._RunClangFormatDiff( - mock.Mock(full=False, dry_run=True, diff=False), ['.'], self._top_dir, - 'HEAD') - self.assertEqual(0, return_value) - def testYapfignoreExplicit(self): self._make_yapfignore(['foo/bar.py', 'foo/bar/baz.py']) files = [