[PATCH] D129311: [clang-format] Update return code

[Sridhar Gopinath via Phabricator via cfe-commits]

sridhar_gopinath marked 3 inline comments as done.
sridhar_gopinath added inline comments.


================
Comment at: clang/tools/clang-format/git-clang-format:201
   if opts.diff:
-    print_diff(old_tree, new_tree)
-  elif opts.diffstat:
-    print_diffstat(old_tree, new_tree)
-  else:
-    changed_files = apply_changes(old_tree, new_tree, force=opts.force,
-                                  patch_mode=opts.patch)
-    if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
-      print('changed files:')
-      for filename in changed_files:
-        print('    %s' % filename)
+    return print_diff(old_tree, new_tree)
+  if opts.diffstat:
----------------
owenpan wrote:
> Actually, doesn't line 175 make sure it will return 0 if there is no diff? Can you open an issue at https://github.com/llvm/llvm-project/issues and give an example where this isn't true?
Created https://github.com/llvm/llvm-project/issues/56736

Line 175 is only checking if there are any relevant files that have been modified. There is an option to only consider a subset of changed files. So this line is checking if there are any changed lines after filtering the relevant files. Then, the tool generates a new git tree for the relevant changes after running clang-format. Line 198 is checking if the old and the new git trees are the same git hashes. When there are code changes, these two hashes won't be the same and we won't hit this case too. Finally, the actual formatting changes are checked by running git diff in line 201. This function call will define the exit code of the program. Currently, that's not being accounted for and the tool always returns 1 after reaching this point.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129311/new/

https://reviews.llvm.org/D129311