Re: [PATCH 2/4] Teach git-add--interactive to accept a file path to patch

Wincent Colaiuta [off-list ref] writes:

- Junio, do you mean to suggest with your comment that when passing
untracked files either directly or indirectly (ie. when passing a dir
containing untracked files) that they should be added (ie. invoked the
"add untracked" subcommand) in addition to running the "patch"
subcommand on the changed files?

What I meant was that if "git add -i" (unrestricted) shows paths
from a set A, "git add -i paths..." should show paths from a
subset of the set A and that subset should be defined with the
existing ls-files pathspec semantics.

For example, if "(a)dd untracked" subcommand shows all untracked
files when "add -i" was invoked without paths limitation, the
restricted form "add -i paths..." would show only untracked paths
that match the given set of patterns.  If "(p)atch" subcommand
shows all modified tracked files when "add -i" was invoked
without paths limitation, the restricted form "add -i paths..."
would show only such modified tracked files whose names match
the given set of patterns.

Implement Junio's suggestion that git-add--interactive should reproduce the
path-limiting semantics of non-interactive git-add.

In otherwords, if "git add -i" (unrestricted) shows paths from a set A,
"git add -i paths..." should show paths from a subset of the set A and that
subset should be defined with the existing ls-files pathspec semantics.

Signed-off-by: Wincent Colaiuta <redacted>
---
 git-add--interactive.perl |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 2bba07d..a5a07bc 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -37,7 +37,7 @@ sub list_untracked {
 		chomp $_;
 		$_;
 	}
-	run_cmd_pipe(qw(git ls-files --others --exclude-standard --), @_);
+	run_cmd_pipe(qw(git ls-files --others --exclude-standard --), @ARGV);
 }
 
 my $status_fmt = '%12s %12s %s';

@@ -56,9 +56,14 @@ sub list_modified {
 	my ($only) = @_;
 	my (%data, @return);
 	my ($add, $del, $adddel, $file);
+	my @tracked = grep {
+		defined run_cmd_pipe(qw(git ls-files
+			                --exclude-standard --), $_)
+	} @ARGV;
+	return if $#tracked == -1 && $#ARGV != -1;
 
 	for (run_cmd_pipe(qw(git diff-index --cached
-			     --numstat --summary HEAD))) {
+			     --numstat --summary HEAD --), @tracked)) {
 		if (($add, $del, $file) =
 		    /^([-\d]+)	([-\d]+)	(.*)/) {
 			my ($change, $bin);

@@ -81,7 +86,7 @@ sub list_modified {
 		}
 	}
 
-	for (run_cmd_pipe(qw(git diff-files --numstat --summary))) {
+	for (run_cmd_pipe(qw(git diff-files --numstat --summary --), @tracked)) {
 		if (($add, $del, $file) =
 		    /^([-\d]+)	([-\d]+)	(.*)/) {
 			if (!exists $data{$file}) {

-- 
1.5.3.6.870.g8799-dirty

El 22/11/2007, a las 1:18, Junio C Hamano escribió:

Wincent Colaiuta [off-list ref] writes:

quoted

- Junio, do you mean to suggest with your comment that when passing
untracked files either directly or indirectly (ie. when passing a dir
containing untracked files) that they should be added (ie. invoked  
the
"add untracked" subcommand) in addition to running the "patch"
subcommand on the changed files?

What I meant was that if "git add -i" (unrestricted) shows paths
from a set A, "git add -i paths..." should show paths from a
subset of the set A and that subset should be defined with the
existing ls-files pathspec semantics.

For example, if "(a)dd untracked" subcommand shows all untracked
files when "add -i" was invoked without paths limitation, the
restricted form "add -i paths..." would show only untracked paths
that match the given set of patterns.  If "(p)atch" subcommand
shows all modified tracked files when "add -i" was invoked
without paths limitation, the restricted form "add -i paths..."
would show only such modified tracked files whose names match
the given set of patterns.

Ok, I've just posted a patch that implements this. This is now going  
somewhat beyond my knowledge of git-diff-index, git-diff-files, and  
git-diff-ls, so I am hoping someone more familiar with the plumbing  
can review this. Basically I'm mostly a porcelain user and have hardly  
touched the plumbing at all during these initial months using Git.

There may be more efficient ways of doing this, but for now I am  
running git-ls-files once for each path parameter to determine whether  
it corresponds to any tracked file(s). This is necessary to weed out  
the untracked paths because passing these to git-diff-index or git- 
diff-files otherwise spits out errors to the stderr.

Cheers,
Wincent

On Wed, Nov 21, 2007 at 04:18:57PM -0800, Junio C Hamano wrote:

What I meant was that if "git add -i" (unrestricted) shows paths
from a set A, "git add -i paths..." should show paths from a
subset of the set A and that subset should be defined with the
existing ls-files pathspec semantics.

Ah, I think that is definitely the right behavior. But it does raise one
more question: is going right into the 'add hunk' interface the correct
behavior, or is that an orthogonal issue?

IOW, do we actually want to support:

  # go to patch menu for each file
  git-add -i -p file1 file2 ...
  # add untracked for each file
  git-add -i -a file1 file2 ...

Which is of course tricky because the '-p' is contextually dependent on
the presence of '-i'.

But perhaps there is no need, since for just these two operations you
can do something like:

  foreach my $file (@ARGV) {
    if(tracked($file)) {
      patch_update_file($file);
    }
    else {
      add_tracked($file);
    }
  }

Are there any other per-file operations that would make sense to start
with? Or might somebody just want to path-limit _without_ starting the
hunk selector?

-Peff