[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbee61aa7b638: [clang-format] Add --staged/--cached option to 
git-clang-format (authored by ortogonal, committed by MyDeveloperDay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes 
are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in 
the
 second  that differ from the first .
@@ -112,6 +113,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',
+ help='format lines in the stage instead of the working dir')
   p.add_argument('--style',
  default=config.get('clangformat.style', None),
  help='passed to clang-format'),
@@ -131,12 +134,14 @@
 
   commits, files = interpret_args(opts.args, dash_dash, opts.commit)
   if len(commits) > 1:
+if opts.staged:
+  die('--staged is not allowed when two commits are given')
 if not opts.diff:
   die('--diff is required when two commits are given')
   else:
 if len(commits) > 2:
   die('at most two commits allowed; %d given' % len(commits))
-  changed_lines = compute_diff_and_extract_lines(commits, files)
+  changed_lines = compute_diff_and_extract_lines(commits, files, opts.staged)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -275,9 +280,9 @@
   return convert_string(stdout.strip())
 
 
-def compute_diff_and_extract_lines(commits, files):
+def compute_diff_and_extract_lines(commits, files, staged):
   """Calls compute_diff() followed by extract_lines()."""
-  diff_process = compute_diff(commits, files)
+  diff_process = compute_diff(commits, files, staged)
   changed_lines = extract_lines(diff_process.stdout)
   diff_process.stdout.close()
   diff_process.wait()
@@ -287,17 +292,21 @@
   return changed_lines
 
 
-def compute_diff(commits, files):
+def compute_diff(commits, files, staged):
   """Return a subprocess object producing the diff from `commits`.
 
   The return value's `stdin` file object will produce a patch with the
-  differences between the working directory and the first commit if a single
-  one was specified, or the difference between both specified commits, filtered
-  on `files` (if non-empty).  Zero context lines are used in the patch."""
+  differences between the working directory (or stage if --staged is used) and
+  the first commit if a single one was specified, or the difference between
+  both specified commits, filtered on `files` (if non-empty).
+  Zero context lines are used in the patch."""
   git_tool = 'diff-index'
+  extra_args = []
   if len(commits) > 1:
 git_tool = 'diff-tree'
-  cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--']
+  elif staged:
+extra_args += ['--cached']
+  cmd = ['git', git_tool, '-p', '-U0'] + extra_args + commits + ['--']
   cmd.extend(files)
   p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
   p.stdin.close()


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in the
 second  that differ from the first .
@@ -112,6 +113,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal added a comment.

@MyDeveloperDay Is everything okay with this? Can you help out with the next 
step in the process?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Mark Lodato via Phabricator via cfe-commits]

lodato accepted this revision.
lodato added a comment.
This revision is now accepted and ready to land.

Oops, I'm sorry. I meant to test it but ran out of time and then forgot to 
click approve. My apologies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@lodato are you blocking this review for a reason?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal added inline comments.



Comment at: clang/tools/clang-format/git-clang-format:140
 if not opts.diff:
   die('--diff is required when two commits are given')
   else:

lodato wrote:
> Does there need to be an equivalent check that --staged requires --diff? 
> Could you test to make sure that works as expected?
Sorry, new to this system. I wrote a reply to this, but it seemed to get lost 
when uploading. Let me try again :)

The check that you can't use `--staged` with two commits is enough. For example 
it you do:
```
git clang-format --diff --staged 53f64aa089c5fd335b1337cab7eaa99d072a25fc 
273e318e1ff0fae58ec7ed248ee3c9c73da8c00e
error: --staged is not allowed when two commits are given
```

But you are allowed to run just `git clang-format --staged`. It you do:
```
$ git add foo.cc
$ git clang-format --staged
changed files:
foo.cc
```
This will run clang-format on the staged changes on foo.cc.

It this what you meant or did I misunderstand you.

Thanks for your review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal updated this revision to Diff 381269.
ortogonal added a comment.

Update description on function `compute_diff`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes 
are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in 
the
 second  that differ from the first .
@@ -112,6 +113,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',
+ help='format lines in the stage instead of the working dir')
   p.add_argument('--style',
  default=config.get('clangformat.style', None),
  help='passed to clang-format'),
@@ -131,12 +134,14 @@
 
   commits, files = interpret_args(opts.args, dash_dash, opts.commit)
   if len(commits) > 1:
+if opts.staged:
+  die('--staged is not allowed when two commits are given')
 if not opts.diff:
   die('--diff is required when two commits are given')
   else:
 if len(commits) > 2:
   die('at most two commits allowed; %d given' % len(commits))
-  changed_lines = compute_diff_and_extract_lines(commits, files)
+  changed_lines = compute_diff_and_extract_lines(commits, files, opts.staged)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -275,9 +280,9 @@
   return convert_string(stdout.strip())
 
 
-def compute_diff_and_extract_lines(commits, files):
+def compute_diff_and_extract_lines(commits, files, staged):
   """Calls compute_diff() followed by extract_lines()."""
-  diff_process = compute_diff(commits, files)
+  diff_process = compute_diff(commits, files, staged)
   changed_lines = extract_lines(diff_process.stdout)
   diff_process.stdout.close()
   diff_process.wait()
@@ -287,17 +292,21 @@
   return changed_lines
 
 
-def compute_diff(commits, files):
+def compute_diff(commits, files, staged):
   """Return a subprocess object producing the diff from `commits`.
 
   The return value's `stdin` file object will produce a patch with the
-  differences between the working directory and the first commit if a single
-  one was specified, or the difference between both specified commits, filtered
-  on `files` (if non-empty).  Zero context lines are used in the patch."""
+  differences between the working directory (or stage if --staged is used) and
+  the first commit if a single one was specified, or the difference between
+  both specified commits, filtered on `files` (if non-empty).
+  Zero context lines are used in the patch."""
   git_tool = 'diff-index'
+  extra_args = []
   if len(commits) > 1:
 git_tool = 'diff-tree'
-  cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--']
+  elif staged:
+extra_args += ['--cached']
+  cmd = ['git', git_tool, '-p', '-U0'] + extra_args + commits + ['--']
   cmd.extend(files)
   p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
   p.stdin.close()


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in the
 second  that differ from the first .
@@ -112,6 +113,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',
+ help='format lines in the stage instead of the working dir')
   

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Mark Lodato via Phabricator via cfe-commits]

lodato requested changes to this revision.
lodato added a comment.
This revision now requires changes to proceed.

Thanks for pushing this through, @ortogonal! The change LGTM, except the two 
minor issues listed.

Name: Mark Lodato
Email: lod...@google.com




Comment at: clang/tools/clang-format/git-clang-format:140
 if not opts.diff:
   die('--diff is required when two commits are given')
   else:

Does there need to be an equivalent check that --staged requires --diff? Could 
you test to make sure that works as expected?



Comment at: clang/tools/clang-format/git-clang-format:299
   The return value's `stdin` file object will produce a patch with the
-  differences between the working directory and the first commit if a single
+  differences between the working/cached directory and the first commit if a 
single
   one was specified, or the difference between both specified commits, filtered

nit: probably more clear to say "working directory (or stage if `staged`)" or 
something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

No problem, I hope your ok if @lodato  gets back to us, I think we should 
probably try and credit them a least a little.

Like I said let us wait a bit for people to have a chance to take a look (I'm 
not in the US hence why I've seen it already),

I doubt there will be a problem as @klimek approved @lodato work (I'm not sure 
why it didn't get landed), but sometimes it just needs someone to be 
persistent, for that I thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal added a comment.

Thanks for all your help (superfast as well)

Yes I need help commiting.

My name: Erik Larsson
My email: karl.erik.lars...@gmail.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: lodato.
MyDeveloperDay added a comment.

@lodato if you are still around (I see no changes since 2017), we may be able 
to add you as a "Co-author" I'm not sure of the policy, (again I would need 
your name and email address)

Co-authored-by: name 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

Much, thank you for seeing this over the line,

This LGTM, lets wait a bit in case any of the others have a comment.

I assume you need help committing this? if so we need your name and your email 
address so we can commit it on your behalf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal added a comment.

Thanks for looking into this!

The feature `--staged` adds the possibility to ONLY run clang format on what is 
staged.

Lets say you have main.cpp where you have done two changes. You stage one of 
them using:

  $ git add -p main.cpp

You now have a state were one of the changes is staged for commit but not the 
other.

  $ git status
  On branch master
  Changes to be committed:
(use "git restore --staged ..." to unstage)
  modified:   main.cpp
  
  Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
  modified:   main.cpp

If you now run `git-clang-format` it will format all changes in main.cpp, but 
at this stage adding `--staged` to git-clang-format lets you only format what 
is staged in that file.

I use this together with a git pre-commit-hook to make sure that all my code 
changes are formatted correctly before I commit. But without the `--staged` 
option I end up in situations were my pre-commit-hook fails because I have part 
of a files staged and a formatting error in the part of the file that is not 
staged.

Does this make the need for `--staged` clearer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

FYI I doubt you'll get a response from the original clang-format code owners as 
they have moved on. But I can take a look

Before giving this the ok, I'd like to understand a little more.

if I make a (bad)formatting change in a file which is not already staged and 
run `git clang-format` it will say

  $ git clang-format
  The following files would be modified but have unstaged changes:
  M   clang/lib/Format/Format.cpp
  Please commit, stage, or stash them first.

If I make a formatting change to a file that is untracked in the current 
directory, then it will NOT format those files

I have to do `git add Format.cpp`  for it tol format my file

  $ git clang-format
  changed files:
  clang/lib/Format/Format.cpp

i.e. I seem to ALWAYS have to stage files in order to have `git clang-format` 
do anything

So I'm kind of unclear as to what doing --staged will do differently? (is it to 
simply ignore the `The following files would be modified but have unstaged 
changes:` warning?

Did I misunderstand?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal updated this revision to Diff 381150.
ortogonal added a comment.

Just refactor it to latest main. I hope this will re-trigger a new build so I 
can investigate if there are still tests that fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes 
are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in 
the
 second  that differ from the first .
@@ -112,6 +113,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',
+ help='format lines in the stage instead of the working dir')
   p.add_argument('--style',
  default=config.get('clangformat.style', None),
  help='passed to clang-format'),
@@ -131,12 +134,14 @@
 
   commits, files = interpret_args(opts.args, dash_dash, opts.commit)
   if len(commits) > 1:
+if opts.staged:
+  die('--staged is not allowed when two commits are given')
 if not opts.diff:
   die('--diff is required when two commits are given')
   else:
 if len(commits) > 2:
   die('at most two commits allowed; %d given' % len(commits))
-  changed_lines = compute_diff_and_extract_lines(commits, files)
+  changed_lines = compute_diff_and_extract_lines(commits, files, opts.staged)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -275,9 +280,9 @@
   return convert_string(stdout.strip())
 
 
-def compute_diff_and_extract_lines(commits, files):
+def compute_diff_and_extract_lines(commits, files, staged):
   """Calls compute_diff() followed by extract_lines()."""
-  diff_process = compute_diff(commits, files)
+  diff_process = compute_diff(commits, files, staged)
   changed_lines = extract_lines(diff_process.stdout)
   diff_process.stdout.close()
   diff_process.wait()
@@ -287,17 +292,20 @@
   return changed_lines
 
 
-def compute_diff(commits, files):
+def compute_diff(commits, files, staged):
   """Return a subprocess object producing the diff from `commits`.
 
   The return value's `stdin` file object will produce a patch with the
-  differences between the working directory and the first commit if a single
+  differences between the working/cached directory and the first commit if a 
single
   one was specified, or the difference between both specified commits, filtered
   on `files` (if non-empty).  Zero context lines are used in the patch."""
   git_tool = 'diff-index'
+  extra_args = []
   if len(commits) > 1:
 git_tool = 'diff-tree'
-  cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--']
+  elif staged:
+extra_args += ['--cached']
+  cmd = ['git', git_tool, '-p', '-U0'] + extra_args + commits + ['--']
   cmd.extend(files)
   p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
   p.stdin.close()


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in the
 second  that differ from the first .
@@ -112,6 +113,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',
+ help='format lines in the stage instead of the working dir')
   p.add_argument('--style',
  default=config.get('clangformat.style', None),
  

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Alexander Lohnau via Phabricator via cfe-commits]

alex1701c added a comment.

Any update on this topic? I really like this feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90996

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

[PATCH] D90996: [clang-format] Add --staged/--cached option to git-clang-format

[Erik Larsson via Phabricator via cfe-commits]

ortogonal created this revision.
ortogonal added reviewers: klimek, Alexander-Shukaev, djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
ortogonal requested review of this revision.

When running git-clang-format in a pre-commit hook it's very useful to be able 
to tell git-clang-format to only look at the --staged/--cached files and not 
the working directory.

Note this patch is a rebase/fork from D41147  
which is a fork of D15465 

This is tested with:

  mkdir /tmp/gcfs
  cd /tmp/gcfs
  
  git init
  
  cat < foo.cc
  int main() {
int x =  1;
return 0;
  }
  EOF
  
  git add foo.cc
  git commit -m 'first commit'
  rm foo.cc
  
  cat < foo.cc
  int main() {
int x =  1;
int y =  2;
int z =  3;
return 0;
  }
  EOF
  
  git add foo.cc
  git commit -m 'second commit'
  sed -i -e 's/1;/10;/' foo.cc
  git add foo.cc
  sed -i -e 's/10;/1;/' foo.cc
  sed -i -e 's/3;/30;/' foo.cc
  
  
  $ git-clang-format --diff
  diff --git a/foo.cc b/foo.cc
  index cb2dbc9..2e1b0e1 100644
  --- a/foo.cc
  +++ b/foo.cc
  @@ -1,6 +1,6 @@
   int main() {
 int x =  1;
 int y =  2;
  -  int z =  30;
  +  int z = 30;
 return 0;
   }
  
  
  $ git-clang-format --diff --staged
  diff --git a/foo.cc b/foo.cc
  index 3ea8c6c..7da0033 100644
  --- a/foo.cc
  +++ b/foo.cc
  @@ -1,5 +1,5 @@
   int main() {
  -  int x =  10;
  +  int x = 10;
 int y =  2;
 int z =  3;
 return 0;
  
  
  $ git-clang-format --diff HEAD~ HEAD
  diff --git a/foo.cc b/foo.cc
  index 7bfdb83..ce6f65e 100644
  --- a/foo.cc
  +++ b/foo.cc
  @@ -1,6 +1,6 @@
   int main() {
 int x =  1;
  -  int y =  2;
  -  int z =  3;
  +  int y = 2;
  +  int z = 3;
 return 0;
   }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90996

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
@@ -32,12 +32,13 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
 If zero or one commits are given, run clang-format on all lines that differ
 between the working directory and , which defaults to HEAD.  Changes 
are
-only applied to the working directory.
+only applied to the working directory, or in the stage/index.
 
 If two commits are given (requires --diff), run clang-format on all lines in 
the
 second  that differ from the first .
@@ -109,6 +110,8 @@
  help='select hunks interactively')
   p.add_argument('-q', '--quiet', action='count', default=0,
  help='print less information')
+  p.add_argument('--staged', '--cached', action='store_true',
+ help='format lines in the stage instead of the working dir')
   p.add_argument('--style',
  default=config.get('clangformat.style', None),
  help='passed to clang-format'),
@@ -128,12 +131,14 @@
 
   commits, files = interpret_args(opts.args, dash_dash, opts.commit)
   if len(commits) > 1:
+if opts.staged:
+  die('--staged is not allowed when two commits are given')
 if not opts.diff:
   die('--diff is required when two commits are given')
   else:
 if len(commits) > 2:
   die('at most two commits allowed; %d given' % len(commits))
-  changed_lines = compute_diff_and_extract_lines(commits, files)
+  changed_lines = compute_diff_and_extract_lines(commits, files, opts.staged)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -268,9 +273,9 @@
   return convert_string(stdout.strip())
 
 
-def compute_diff_and_extract_lines(commits, files):
+def compute_diff_and_extract_lines(commits, files, staged):
   """Calls compute_diff() followed by extract_lines()."""
-  diff_process = compute_diff(commits, files)
+  diff_process = compute_diff(commits, files, staged)
   changed_lines = extract_lines(diff_process.stdout)
   diff_process.stdout.close()
   diff_process.wait()
@@ -280,17 +285,20 @@
   return changed_lines
 
 
-def compute_diff(commits, files):
+def compute_diff(commits, files, staged):
   """Return a subprocess object producing the diff from `commits`.
 
   The return value's `stdin` file object will produce a patch with the
-  differences between the working directory and the first commit if a single
+  differences between the working/cached directory and the first commit if a 
single
   one was specified, or the difference between both specified commits, filtered
   on `files` (if non-empty).  Zero context lines are used in the patch."""
   git_tool = 'diff-index'
+  extra_args = []
   if len(commits) > 1:
 git_tool = 'diff-tree'
-  cmd = ['git',