[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

[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] D41147: git-clang-format: Add new --staged option.

[Mark Lodato via Phabricator via cfe-commits]

lodato updated this revision to Diff 126793.
lodato added a comment.

Update after change to https://reviews.llvm.org/D41145 that fixed `len(commits) 
> 2` case.


https://reviews.llvm.org/D41147

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format

Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -33,11 +33,12 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
-Run clang-format on all modified lines in the working directory or in a given
-commit.
+Run clang-format on all modified lines in the working directory, in a given
+commit, or in the stage/index.
 
 Forms:
 
@@ -57,6 +58,10 @@
 Run clang-format on all lines in  that differ from .
 Requires --diff.
 
+git clang-format [] --staged --diff
+Run clang-format on all lines in the git stage/index that differ from
+, which defaults to HEAD. Requires --diff.
+
 In all of the forms above, ... can be used to limit what files are
 affected, using the same syntax as `git diff`. Use `--` to disambiguate between
 files and commits.
@@ -126,6 +131,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'),
@@ -144,7 +151,10 @@
   del opts.quiet
 
   source, dest, files = interpret_args(opts.args, dash_dash,
-   default_commit=opts.commit)
+   default_commit=opts.commit,
+   staged=opts.staged)
+  if isinstance(dest, Stage) and not opts.diff:
+die('--diff is required when --staged is used')
   if isinstance(dest, Revision) and not opts.diff:
 die('--diff is required when two commits are given')
   changed_lines = dest.compute_diff_from(source, files)
@@ -207,7 +217,7 @@
   return out
 
 
-def interpret_args(args, dash_dash, default_commit):
+def interpret_args(args, dash_dash, default_commit, staged):
   """Interpret `args` as "[commits] [--] [files]" and return (src, dest, files).
 
   It is assumed that "--" and everything that follows has been removed from
@@ -252,7 +262,11 @@
   if len(commits) > 2:
 die('at most two commits allowed; %d given' % len(commits))
   elif len(commits) == 2:
+if staged:
+  die('--staged is not allowed when two commits are given')
 return Revision(commits[0]), Revision(commits[1]), files
+  elif staged:
+return Revision(commits[0]), Stage(), files
   else:
 return Revision(commits[0]), Workdir(), files
 
@@ -283,7 +297,7 @@
 
 
 class TreeLocation (object):
-  """Represents either a commit or the working directory.
+  """Represents either a commit, the stage, or the working directory.
 
   Do not use this directly. Instead, use one of the subclasses."""
 
@@ -439,6 +453,24 @@
   is_workdir = True
 
 
+class Stage (TreeLocation):
+  """Represents the git stage, a.k.a. the index."""
+
+  def create_tree(self, unused_filenames):
+return run('git', 'write-tree')
+
+  def _file_mode(self, filename):
+stdout = run('git', 'ls-files', '--stage', filename)
+# note: there is an optional  as the first element.
+return int(stdout.split()[-4], 8)
+
+  def _compute_diff_from_base_command(self, source):
+return ['git', 'diff-index', '-p', '-U0', '--cached', source.revision]
+
+  def _blob_name(self, filename):
+return ':%s' % filename
+
+
 class Revision (TreeLocation):
   """Represents a specific revision, a.k.a. a commit."""
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Oops! Fixed. Thanks for catching this!


https://reviews.llvm.org/D41145



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

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

[Mark Lodato via Phabricator via cfe-commits]

lodato updated this revision to Diff 126792.
lodato added a comment.

Re-add check for `len(commits) > 2`.


https://reviews.llvm.org/D41145

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format

Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -36,12 +36,30 @@
 usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
 
 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.
+Run clang-format on all modified lines in the working directory or in a given
+commit.
 
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
+Forms:
+
+git clang-format []
+Run clang-format on all lines in the working directory that differ from
+, which defaults to HEAD. Changes are written in-place.
+
+git clang-format [] --diff
+Same as first form, but print the diff instead of writing changes
+in-place.
+
+git clang-format [] --patch
+Same as first form, but interactively choose hunks to apply, a la `git
+add -p`.
+
+git clang-format   --diff
+Run clang-format on all lines in  that differ from .
+Requires --diff.
+
+In all of the forms above, ... can be used to limit what files are
+affected, using the same syntax as `git diff`. Use `--` to disambiguate between
+files and commits.
 
 The following git-config settings set the default of the corresponding option:
   clangFormat.binary
@@ -125,14 +143,11 @@
   opts.verbose -= opts.quiet
   del opts.quiet
 
-  commits, files = interpret_args(opts.args, dash_dash, opts.commit)
-  if len(commits) > 1:
-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)
+  source, dest, files = interpret_args(opts.args, dash_dash,
+   default_commit=opts.commit)
+  if isinstance(dest, Revision) and not opts.diff:
+die('--diff is required when two commits are given')
+  changed_lines = dest.compute_diff_from(source, files)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -152,17 +167,10 @@
   # The computed diff outputs absolute paths, so we must cd before accessing
   # those files.
   cd_to_toplevel()
-  if len(commits) > 1:
-old_tree = commits[1]
-new_tree = run_clang_format_and_save_to_tree(changed_lines,
- revision=commits[1],
- binary=opts.binary,
- style=opts.style)
-  else:
-old_tree = create_tree_from_workdir(changed_lines)
-new_tree = run_clang_format_and_save_to_tree(changed_lines,
- binary=opts.binary,
- style=opts.style)
+  old_tree = dest.create_tree(changed_lines)
+  new_tree = dest.run_clang_format_and_save_to_tree(changed_lines,
+binary=opts.binary,
+style=opts.style)
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
@@ -172,6 +180,7 @@
   elif opts.diff:
 print_diff(old_tree, new_tree)
   else:
+assert isinstance(dest, Workdir)
 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:
@@ -199,15 +208,20 @@
 
 
 def interpret_args(args, dash_dash, default_commit):
-  """Interpret `args` as "[commits] [--] [files]" and return (commits, files).
+  """Interpret `args` as "[commits] [--] [files]" and return (src, dest, files).
 
   It is assumed that "--" and everything that follows has been removed from
   args and placed in `dash_dash`.
 
   If "--" is present (i.e., `dash_dash` is non-empty), the arguments to its
   left (if present) are taken as commits.  Otherwise, the arguments are checked
   from left to right if they are commits or files.  If commits are not given,
-  a list with `default_commit` is used."""
+  a list with `default_commit` is used.
+
+  Return value is `(src, dest, files)`, where `src` and `dest` are TreeLocation
+  objects and `files` is a (possibly empty) list of filenames.
+  """
+  # First, get the list 

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Oh, and I meant to start with: I'm so sorry for the extremely long delay. I was 
swamped with work before then I forgot about this. Please know that I 
appreciate your effort here and that I didn't mean to blow you off.

Best regards, Mark


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

I think the simplest solution to those problems is to require `--diff`. An 
alternative is to write the changes directly to the index without touching the 
working directory, but that would require some flag because the behavior is 
unintuitive, and the implementation would be complicated enough to warrant its 
own patch.

I reimplemented your patch in https://reviews.llvm.org/D41147 based on a 
significant refactoring, which I hope makes the code more clear.


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

[PATCH] D41147: git-clang-format: Add new --staged option.

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek, Alexander-Shukaev.

This new mode, which requires --diff, operates very similarly to the two 
 mode, except that the stage is formatted instead of the second commit.

The main intent of this feature is to use in pre-commit hooks so that you can 
ensure that all commits are properly formatted. Without this, the commit hook 
would have to format the working directory which is not necessarily what will 
be committed.

This mode requires --diff because otherwise there would be no place for the 
update to be written to: writing to the index would blow away changes, and 
writing to the working directory would be incorrect in case the working 
directory differs from the index.

Note: this patch was forked from https://reviews.llvm.org/D15465.

Testing of all three modes:

  # setup
  $ mkdir tmp
  $ cd tmp
  $ git init
  $ cat > foo.cc
  int main() {
int x =  1;
return 0;
  }
  EOF
  $ git add foo.cc
  $ git commit -m 'initial 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 'commit 2'
  $ 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;
   }




https://reviews.llvm.org/D41147

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format

Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -33,11 +33,12 @@
 import subprocess
 import sys
 
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
+usage = ('git clang-format [OPTIONS] [] [|--staged] '
+ '[--] [...]')
 
 desc = '''
-Run clang-format on all modified lines in the working directory or in a given
-commit.
+Run clang-format on all modified lines in the working directory, in a given
+commit, or in the stage/index.
 
 Forms:
 
@@ -57,6 +58,10 @@
 Run clang-format on all lines in  that differ from .
 Requires --diff.
 
+git clang-format [] --staged --diff
+Run clang-format on all lines in the git stage/index that differ from
+, which defaults to HEAD. Requires --diff.
+
 In all of the forms above, ... can be used to limit what files are
 affected, using the same syntax as `git diff`. Use `--` to disambiguate between
 files and commits.
@@ -126,6 +131,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'),
@@ -144,7 +151,10 @@
   del opts.quiet
 
   source, dest, files = interpret_args(opts.args, dash_dash,
-   default_commit=opts.commit)
+   default_commit=opts.commit,
+   staged=opts.staged)
+  if isinstance(dest, Stage) and not opts.diff:
+die('--diff is required when --staged is used')
   if isinstance(dest, Revision) and not opts.diff:
 die('--diff is required when two commits are given')
   changed_lines = dest.compute_diff_from(source, files)
@@ -207,7 +217,7 @@
   return out
 
 
-def interpret_args(args, dash_dash, default_commit):
+def interpret_args(args, dash_dash, default_commit, staged):
   """Interpret `args` as "[commits] [--] [files]" and return (src, dest, files).
 
   It is assumed that "--" and everything that follows has been removed from
@@ -250,7 +260,11 @@
 
   assert len(commits) != 0
   if len(commits) >= 2:
+if staged:
+  die('--staged is not allowed when two commits are given')
 return Revision(commits[0]), Revision(commits[1]), files
+  elif staged:
+return Revision(commits[0]), 

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek, Alexander-Shukaev.

The only user-visible change is rewriting of the --help message to make the 
different modes more clear.

Internally, this is a significant refactoring to make the source and 
destination of the diff to be more clear by using subclasses rather than 
conditionals. The purpose is to support the next patch in this series, which 
adds support for a --staged flag. But even if that does not go through, the 
intent is for this code to be a readability improvement.


https://reviews.llvm.org/D41145

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format

Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -36,12 +36,30 @@
 usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
 
 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.
+Run clang-format on all modified lines in the working directory or in a given
+commit.
 
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
+Forms:
+
+git clang-format []
+Run clang-format on all lines in the working directory that differ from
+, which defaults to HEAD. Changes are written in-place.
+
+git clang-format [] --diff
+Same as first form, but print the diff instead of writing changes
+in-place.
+
+git clang-format [] --patch
+Same as first form, but interactively choose hunks to apply, a la `git
+add -p`.
+
+git clang-format   --diff
+Run clang-format on all lines in  that differ from .
+Requires --diff.
+
+In all of the forms above, ... can be used to limit what files are
+affected, using the same syntax as `git diff`. Use `--` to disambiguate between
+files and commits.
 
 The following git-config settings set the default of the corresponding option:
   clangFormat.binary
@@ -125,14 +143,11 @@
   opts.verbose -= opts.quiet
   del opts.quiet
 
-  commits, files = interpret_args(opts.args, dash_dash, opts.commit)
-  if len(commits) > 1:
-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)
+  source, dest, files = interpret_args(opts.args, dash_dash,
+   default_commit=opts.commit)
+  if isinstance(dest, Revision) and not opts.diff:
+die('--diff is required when two commits are given')
+  changed_lines = dest.compute_diff_from(source, files)
   if opts.verbose >= 1:
 ignored_files = set(changed_lines)
   filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -152,17 +167,10 @@
   # The computed diff outputs absolute paths, so we must cd before accessing
   # those files.
   cd_to_toplevel()
-  if len(commits) > 1:
-old_tree = commits[1]
-new_tree = run_clang_format_and_save_to_tree(changed_lines,
- revision=commits[1],
- binary=opts.binary,
- style=opts.style)
-  else:
-old_tree = create_tree_from_workdir(changed_lines)
-new_tree = run_clang_format_and_save_to_tree(changed_lines,
- binary=opts.binary,
- style=opts.style)
+  old_tree = dest.create_tree(changed_lines)
+  new_tree = dest.run_clang_format_and_save_to_tree(changed_lines,
+binary=opts.binary,
+style=opts.style)
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
@@ -172,6 +180,7 @@
   elif opts.diff:
 print_diff(old_tree, new_tree)
   else:
+assert isinstance(dest, Workdir)
 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:
@@ -199,15 +208,20 @@
 
 
 def interpret_args(args, dash_dash, default_commit):
-  """Interpret `args` as "[commits] [--] [files]" and return (commits, files).
+  """Interpret `args` as "[commits] [--] [files]" and return (src, dest, files).
 
   It is assumed that "--" and everything that follows has been removed from
   args and placed in `dash_dash`.
 
   If "--" is present (i.e., `dash_dash` is 

[PATCH] D41129: Fix `git-clang-format `.

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Yeah, I was incorrect. The feature worked fine. I don't know what I was 
thinking.


https://reviews.llvm.org/D41129



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

[PATCH] D41129: Fix `git-clang-format `.

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Actually the old code might have been working fine. Please hold off on 
reviewing.


https://reviews.llvm.org/D41129



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

[PATCH] D41130: git-clang-format: cleanup: Use run() when possible.

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek.

This makes the code a bit easier to understand. Also add a docstring to `run()`.

Note: This means that we read the entire index into memory when calling `git 
update-index`, whereas previously we would send the data line-by-line. Git 
already loads the entire index into memory anyway* so this should not cause a 
regression.

- To my knowledge.


https://reviews.llvm.org/D41130

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format


Index: 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -356,12 +356,9 @@
   def index_info_generator():
 for filename, line_ranges in iteritems(changed_lines):
   if revision:
-git_metadata_cmd = ['git', 'ls-tree',
-'%s:%s' % (revision, os.path.dirname(filename)),
-os.path.basename(filename)]
-git_metadata = subprocess.Popen(git_metadata_cmd, 
stdin=subprocess.PIPE,
-stdout=subprocess.PIPE)
-stdout = git_metadata.communicate()[0]
+stdout = run('git', 'ls-tree',
+ '%s:%s' % (revision, os.path.dirname(filename)),
+ os.path.basename(filename))
 mode = oct(int(stdout.split()[0], 8))
   else:
 mode = oct(os.stat(filename).st_mode)
@@ -384,14 +381,9 @@
   --index-info", such as "".  Any other mode
   is invalid."""
   assert mode in ('--stdin', '--index-info')
-  cmd = ['git', 'update-index', '--add', '-z', mode]
   with temporary_index_file():
-p = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-for line in input_lines:
-  p.stdin.write(to_bytes('%s\0' % line))
-p.stdin.close()
-if p.wait() != 0:
-  die('`%s` failed' % ' '.join(cmd))
+run('git', 'update-index', '--add', '-z', mode,
+stdin=to_bytes(''.join('%s\0' % line for line in input_lines)))
 tree_id = run('git', 'write-tree')
 return tree_id
 
@@ -522,6 +514,7 @@
 
 
 def run(*args, **kwargs):
+  """Runs the given command and returns stdout; exits on command failure."""
   stdin = kwargs.pop('stdin', '')
   verbose = kwargs.pop('verbose', True)
   strip = kwargs.pop('strip', True)


Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -356,12 +356,9 @@
   def index_info_generator():
 for filename, line_ranges in iteritems(changed_lines):
   if revision:
-git_metadata_cmd = ['git', 'ls-tree',
-'%s:%s' % (revision, os.path.dirname(filename)),
-os.path.basename(filename)]
-git_metadata = subprocess.Popen(git_metadata_cmd, stdin=subprocess.PIPE,
-stdout=subprocess.PIPE)
-stdout = git_metadata.communicate()[0]
+stdout = run('git', 'ls-tree',
+ '%s:%s' % (revision, os.path.dirname(filename)),
+ os.path.basename(filename))
 mode = oct(int(stdout.split()[0], 8))
   else:
 mode = oct(os.stat(filename).st_mode)
@@ -384,14 +381,9 @@
   --index-info", such as "".  Any other mode
   is invalid."""
   assert mode in ('--stdin', '--index-info')
-  cmd = ['git', 'update-index', '--add', '-z', mode]
   with temporary_index_file():
-p = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-for line in input_lines:
-  p.stdin.write(to_bytes('%s\0' % line))
-p.stdin.close()
-if p.wait() != 0:
-  die('`%s` failed' % ' '.join(cmd))
+run('git', 'update-index', '--add', '-z', mode,
+stdin=to_bytes(''.join('%s\0' % line for line in input_lines)))
 tree_id = run('git', 'write-tree')
 return tree_id
 
@@ -522,6 +514,7 @@
 
 
 def run(*args, **kwargs):
+  """Runs the given command and returns stdout; exits on command failure."""
   stdin = kwargs.pop('stdin', '')
   verbose = kwargs.pop('verbose', True)
   strip = kwargs.pop('strip', True)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41129: Fix `git-clang-format `.

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek.

This feature had never worked at all because the code incorrectly used 
<`commit2>` for both the "old" and "new", so the diff was always empty! Now we 
correctly use `` where we should and it works fine.


https://reviews.llvm.org/D41129

Files:
  google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format


Index: 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ 
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -153,7 +153,7 @@
   # those files.
   cd_to_toplevel()
   if len(commits) > 1:
-old_tree = commits[1]
+old_tree = commits[0]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
  revision=commits[1],
  binary=opts.binary,


Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -153,7 +153,7 @@
   # those files.
   cd_to_toplevel()
   if len(commits) > 1:
-old_tree = commits[1]
+old_tree = commits[0]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
  revision=commits[1],
  binary=opts.binary,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Mark Lodato via Phabricator via cfe-commits]

lodato added a comment.

Sorry, I have been very busy with other work so I haven't had a chance to 
follow along. (I don't work on LLVM team - I just contributed this script.)

I'll try to carve out some time to review within the next week.


Repository:
  rL LLVM

https://reviews.llvm.org/D15465



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

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

[Mark Lodato via Phabricator via cfe-commits]

lodato resigned from this revision.
lodato removed a reviewer: lodato.
lodato added a comment.

I know nothing about the C++ code. I only know the git-clang-format script.


https://reviews.llvm.org/D27377



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