[PATCH] D84375: [git-clang-format] Add --diffstat parameter

[Roland via Phabricator via cfe-commits]

roligugus created this revision.
roligugus added reviewers: klimek, aheejin.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

[git-clang-format][PR46815] Add diffstat functionality

Adding a --diffstat parameter to git-clang-format that essentially uses git 
diff --stat, i.e. lists the files needing
formatting. This is useful for CI integration or manual usage where one wants 
to list the files not properly formatted.

I use it for the Suricata project's github action (CI) integration that 
verifies proper formatting of a pull request 
according to project guidelines where it's very helpful to say which files are 
not properly formatted. I find the list
of files much more useful than e.g. showing the diff in this case using 
git-clang-format --diff.

An alternative would be to take an additional parameter to diff, e.g. 
git-clang-format --diff --stat

The goal is not to provide the whole git diff --stat=... parameter 
functionality, just plain git diff --stat.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84375

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -98,6 +98,8 @@
  help='default commit to use if none is specified'),
   p.add_argument('--diff', action='store_true',
  help='print a diff instead of applying the changes')
+  p.add_argument('--diffstat', action='store_true',
+ help='print a diffstat instead of applying the changes')
   p.add_argument('--extensions',
  default=config.get('clangformat.extensions',
 default_extensions),
@@ -172,6 +174,8 @@
   print('clang-format did not modify any files')
   elif 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)
@@ -494,6 +498,17 @@
   subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
  '--'])
 
+def print_diffstat(old_tree, new_tree):
+  """Print the diffstat between the two trees to stdout."""
+  # We use the porcelain 'diff' and not plumbing 'diff-tree' because the output
+  # is expected to be viewed by the user, and only the former does nice things
+  # like color and pagination.
+  #
+  # We also only print modified files since `new_tree` only contains the files
+  # that were modified, so unmodified files would show as deleted without the
+  # filter.
+  subprocess.check_call(['git', 'diff', '--diff-filter=M', '--stat', old_tree, 
new_tree,
+ '--'])
 
 def apply_changes(old_tree, new_tree, force=False, patch_mode=False):
   """Apply the changes in `new_tree` to the working directory.


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -98,6 +98,8 @@
  help='default commit to use if none is specified'),
   p.add_argument('--diff', action='store_true',
  help='print a diff instead of applying the changes')
+  p.add_argument('--diffstat', action='store_true',
+ help='print a diffstat instead of applying the changes')
   p.add_argument('--extensions',
  default=config.get('clangformat.extensions',
 default_extensions),
@@ -172,6 +174,8 @@
   print('clang-format did not modify any files')
   elif 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)
@@ -494,6 +498,17 @@
   subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
  '--'])
 
+def print_diffstat(old_tree, new_tree):
+  """Print the diffstat between the two trees to stdout."""
+  # We use the porcelain 'diff' and not plumbing 'diff-tree' because the output
+  # is expected to be viewed by the user, and only the former does nice things
+  # like color and pagination.
+  #
+  # We also only print modified files since `new_tree` only contains the files
+  # that were modified, so unmodified files would show as deleted without the
+  # filter.
+  subprocess.check_call(['git', 'diff', '--diff-filter=M', '--stat', old_tree, new_tree,
+ '--'])
 
 def apply_changes(old_tree, new_tree, force=False, patch_mode=False):
   """Apply the changes in `new_tree` to the working directory.
___
cfe-commits mailing lis

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

[Roland via Phabricator via cfe-commits]

roligugus added a project: clang-format.
roligugus marked an inline comment as done.
roligugus added inline comments.



Comment at: clang/tools/clang-format/git-clang-format:101
  help='print a diff instead of applying the changes')
+  p.add_argument('--diffstat', action='store_true',
+ help='print a diffstat instead of applying the changes')

As mentioned in the description, an alternative would be to add a `--stat` 
parameter and hand that to `print_diff(`). E.g. user would call it then with 
`git-clang-format --diff --stat`.

Would result in less code duplication of `print_diff()` vs `print_diffstat()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84375



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

[Roland via Phabricator via cfe-commits]

roligugus requested review of this revision.
roligugus added a comment.

@JakeMerdichAMD, @MyDeveloperDay
Sorry to bother with a re-review. Did not know how to ask this.

Jake accepted this patch last August and it has been sitting in "ready to land" 
since then. I was assuming that it'll automatically make it into the main line 
after the acceptance. My bad, I should have read the policy and practices 
better.

Do you mind re-accepting and then committing this? I don't have any commit 
rights.

Sorry, I should have followed up way earlier. I was finally getting around to 
this and wanted to take our local patch out and was assuming this was in the 
latest clang, but it isn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84375

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111853: [NFC][Interpreter] Remove unused CompilerInvocation

[Roland via Phabricator via cfe-commits]

roligugus created this revision.
roligugus requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111853

Files:
  clang/lib/Interpreter/Interpreter.cpp


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -144,7 +144,6 @@
   // driver to construct.
   ClangArgv.push_back("<<< inputs >>>");
 
-  CompilerInvocation Invocation;
   // Buffer diagnostics from argument parsing so that we can output them using 
a
   // well formed diagnostic object.
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());


Index: clang/lib/Interpreter/Interpreter.cpp
===
--- clang/lib/Interpreter/Interpreter.cpp
+++ clang/lib/Interpreter/Interpreter.cpp
@@ -144,7 +144,6 @@
   // driver to construct.
   ClangArgv.push_back("<<< inputs >>>");
 
-  CompilerInvocation Invocation;
   // Buffer diagnostics from argument parsing so that we can output them using a
   // well formed diagnostic object.
   IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

[Roland via Phabricator via cfe-commits]

roligugus updated this revision to Diff 379892.
roligugus added a comment.

[git-clang-format] Rebase D84375  on latest 
main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84375

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -99,6 +99,8 @@
  help='default commit to use if none is specified'),
   p.add_argument('--diff', action='store_true',
  help='print a diff instead of applying the changes')
+  p.add_argument('--diffstat', action='store_true',
+ help='print a diffstat instead of applying the changes')
   p.add_argument('--extensions',
  default=config.get('clangformat.extensions',
 default_extensions),
@@ -176,6 +178,8 @@
   print('clang-format did not modify any files')
   elif 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)
@@ -506,6 +510,17 @@
   subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
  '--'])
 
+def print_diffstat(old_tree, new_tree):
+  """Print the diffstat between the two trees to stdout."""
+  # We use the porcelain 'diff' and not plumbing 'diff-tree' because the output
+  # is expected to be viewed by the user, and only the former does nice things
+  # like color and pagination.
+  #
+  # We also only print modified files since `new_tree` only contains the files
+  # that were modified, so unmodified files would show as deleted without the
+  # filter.
+  subprocess.check_call(['git', 'diff', '--diff-filter=M', '--stat', old_tree, 
new_tree,
+ '--'])
 
 def apply_changes(old_tree, new_tree, force=False, patch_mode=False):
   """Apply the changes in `new_tree` to the working directory.


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -99,6 +99,8 @@
  help='default commit to use if none is specified'),
   p.add_argument('--diff', action='store_true',
  help='print a diff instead of applying the changes')
+  p.add_argument('--diffstat', action='store_true',
+ help='print a diffstat instead of applying the changes')
   p.add_argument('--extensions',
  default=config.get('clangformat.extensions',
 default_extensions),
@@ -176,6 +178,8 @@
   print('clang-format did not modify any files')
   elif 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)
@@ -506,6 +510,17 @@
   subprocess.check_call(['git', 'diff', '--diff-filter=M', old_tree, new_tree,
  '--'])
 
+def print_diffstat(old_tree, new_tree):
+  """Print the diffstat between the two trees to stdout."""
+  # We use the porcelain 'diff' and not plumbing 'diff-tree' because the output
+  # is expected to be viewed by the user, and only the former does nice things
+  # like color and pagination.
+  #
+  # We also only print modified files since `new_tree` only contains the files
+  # that were modified, so unmodified files would show as deleted without the
+  # filter.
+  subprocess.check_call(['git', 'diff', '--diff-filter=M', '--stat', old_tree, new_tree,
+ '--'])
 
 def apply_changes(old_tree, new_tree, force=False, patch_mode=False):
   """Apply the changes in `new_tree` to the working directory.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84375: [git-clang-format] Add --diffstat parameter

[Roland via Phabricator via cfe-commits]

roligugus added a comment.

@MyDeveloperDay Thanks for the follow-up! I've rebased on latest main.

Out of curiosity, as I am trying to wrap my head around the llvm workflow: 
Could I `arc land ...` myself once you sign off with "ready to land" even if I 
don't have llvm commit rights? If so, please let me know and I'll do that.

Otherwise, I've set the author on my commit, but if you need to set it: `git 
commit --amend --author="Roland Fischer "`

Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84375

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits