From d8b56a6074398032bc49951979a4390a78c30d56 Mon Sep 17 00:00:00 2001 From: "dcheng@chromium.org" Date: Thu, 4 Apr 2013 17:48:18 +0000 Subject: [PATCH] Have run_tool.py automatically invoke clang-format-diff.py if available. This makes for nicer code and removes the extra post-processing step. It also provides more granular warnings; simply returning the formatter across the entire diff will provide cryptic errors like missing { at line 540. BUG= Review URL: https://codereview.chromium.org/13605003 git-svn-id: http://src.chromium.org/svn/trunk/src/tools/clang@192359 4ff67af0-8c30-449e-8e8b-ad334ec8d88c --- empty_string/tests/test-expected.cc | 13 ++++++------- scripts/run_tool.py | 22 ++++++++++++++++++---- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/empty_string/tests/test-expected.cc b/empty_string/tests/test-expected.cc index e9156255..7fd96134 100644 --- a/empty_string/tests/test-expected.cc +++ b/empty_string/tests/test-expected.cc @@ -12,17 +12,17 @@ void TestDeclarations() { std::string a, b("abc"), c; } // Tests for std::string allocated with new. void TestNew() { std::string* a = new std::string, - *b = new std::string("abc"), - *c = new std::string, - *d = new std::string(); + *b = new std::string("abc"), + *c = new std::string, + *d = new std::string(); } // Tests for std::string construction in initializer lists. class TestInitializers { public: - TestInitializers() {} - TestInitializers(bool) {} - TestInitializers(double) : b("cat"), c() {} + TestInitializers() {} + TestInitializers(bool) {} + TestInitializers(double) : b("cat"), c() {} private: std::string a; @@ -43,4 +43,3 @@ void TestWideTemporaries(const std::wstring& reference_argument, TestWideTemporaries(std::wstring(), std::wstring()); TestWideTemporaries(std::wstring(), std::wstring()); } - diff --git a/scripts/run_tool.py b/scripts/run_tool.py index 597c825a..903235d4 100755 --- a/scripts/run_tool.py +++ b/scripts/run_tool.py @@ -183,11 +183,14 @@ def __ProcessResult(self, result): sys.stdout.flush() -def _ApplyEdits(edits): +def _ApplyEdits(edits, clang_format_diff_path): """Apply the generated edits. Args: edits: A dict mapping filenames to Edit instances that apply to that file. + clang_format_diff_path: Path to the clang-format-diff.py helper to help + automatically reformat diffs to avoid style violations. Pass None if the + clang-format step should be skipped. """ edit_count = 0 for k, v in edits.iteritems(): @@ -210,6 +213,10 @@ def _ApplyEdits(edits): f.seek(0) f.truncate() f.write(contents) + if clang_format_diff_path: + if subprocess.call('git diff -U0 %s | python %s -style=Chromium' % ( + k, clang_format_diff_path), shell=True) != 0: + print 'clang-format failed for %s' % k print 'Applied %d edits to %d files' % (edit_count, len(edits)) @@ -266,6 +273,14 @@ def main(argv): print ' ... can be used to filter what files are edited' return 1 + clang_format_diff_path = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + '../../../third_party/llvm/tools/clang/tools/clang-format', + 'clang-format-diff.py') + # TODO(dcheng): Allow this to be controlled with a flag as well. + if not os.path.isfile(clang_format_diff_path): + clang_format_diff_path = None + filenames = frozenset(_GetFilesFromGit(argv[2:])) # Filter out files that aren't C/C++/Obj-C/Obj-C++. extensions = frozenset(('.c', '.cc', '.m', '.mm')) @@ -277,9 +292,8 @@ def main(argv): # useful to modify files that aren't under source control--typically, these # are generated files or files in a git submodule that's not part of Chromium. _ApplyEdits({k : v for k, v in dispatcher.edits.iteritems() - if k in filenames}) - # TODO(dcheng): Consider clang-formatting the result to avoid egregious style - # violations. + if k in filenames}, + clang_format_diff_path) if dispatcher.failed_count != 0: return 2 return 0