[PATCH] teaching git-add--interactive to accept a file param

[PATCH] teaching git-add--interactive to accept a file param

[Wincent Colaiuta]

Back in October a thread came up, "Proposed git mv behavioral change",
and I inadvertently suggested a new feature for git-add--interactive;
the ability to supply an optional file path parameter and jump straight
to the "patch" subcommand for that file:

<http://kerneltrap.org/mailarchive/git/2007/10/19/348229>

This small patch series implements that suggestion:

      Refactor patch_update_cmd
      Teach git-add--interactive to accept a file path to patch
      Teach builtin-add to pass a path argument to git-add--interactive
      Document optional file parameter to git-add--interactive

It applies against the current master. I've not yet submitted many
patches to Git, so please be gentle!
                        
Documentation/git-add.txt |    5 ++++-
builtin-add.c             |   16 ++++++++++------
commit.h                  |    2 +-
git-add--interactive.perl |   11 +++++++++--
4 files changed, 24 insertions(+), 10 deletions(-)

[PATCH 1/4] Refactor patch_update_cmd

[Wincent Colaiuta]

Split patch_update_cmd into two functions, one to prompt the user for
a path to patch and another to do the actual work given that file path.
This lays the groundwork for a future commit which will teach
git-add--interactive to accept a path parameter and jump directly to
the patch subcommand for that path, bypassing the interactive prompt.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-add--interactive.perl |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 0317ad9..fb1e92a 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -564,10 +564,12 @@ sub patch_update_cmd {
 				     IMMEDIATE => 1,
 				     HEADER => $status_head, },
 				   @mods);
-	return if (!$it);
+	patch_update_file($it->{VALUE}) if ($it);
+}
 
+sub patch_update_file {
 	my ($ix, $num);
-	my $path = $it->{VALUE};
+	my $path = shift;
 	my ($head, @hunk) = parse_diff($path);
 	for (@{$head->{TEXT}}) {
 		print;
-- 
1.5.3.5.737.gdee1b

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

[Wincent Colaiuta]

If supplied a single file path parameter the git-add--interactive script
now bypasses the command loop and jumps straight to the patch subcommand
using the passed path. After returning from the subcommand the main
command loop is entered. If a non-resolvable path is supplied the
operation is a no-op and the command loop is entered.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-add--interactive.perl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..8f21c03 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -803,6 +803,11 @@ sub main_loop {
 	}
 }
 
+die "add --interactive may take only 1 optional parameter" if ($#ARGV > 0);
 refresh();
+if ($#ARGV == 0) {
+	patch_update_file($ARGV[0]);
+}
 status_cmd();
 main_loop();
+
-- 
1.5.3.5.737.gdee1b

[PATCH 3/4] Teach builtin-add to pass a path argument to git-add--interactive

[Wincent Colaiuta]

The previous patch in the series taught git-add--interactive to handle
a single optional path argument. This patch teaches builtin-add to pass
this argument through to the script.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 builtin-add.c |   16 ++++++++++------
 commit.h      |    2 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index cf815a0..278c02e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -135,9 +135,9 @@ static void refresh(int verbose, const char **pathspec)
         free(seen);
 }
 
-int interactive_add(void)
+int interactive_add(const char *path)
 {
-	const char *argv[2] = { "add--interactive", NULL };
+	const char *argv[3] = { "add--interactive", path, NULL };
 
 	return run_command_v_opt(argv, RUN_GIT_CMD);
 }
@@ -163,16 +163,20 @@ static struct option builtin_add_options[] = {
 
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
-	int i, newfd, orig_argc = argc;
+	int i, newfd = argc;
 	const char **pathspec;
 	struct dir_struct dir;
 
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
 	if (add_interactive) {
-		if (add_interactive != 1 || orig_argc != 2)
-			die("add --interactive does not take any parameters");
-		exit(interactive_add());
+		if (argc > 1)
+			die("add --interactive may take only 1 optional "
+			    "parameter");
+		else if (argc == 1)
+			exit(interactive_add(argv[0]));
+		else
+			exit(interactive_add(NULL));
 	}
 
 	git_config(git_default_config);
diff --git a/commit.h b/commit.h
index aa67986..03a6ec5 100644
--- a/commit.h
+++ b/commit.h
@@ -113,7 +113,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
 
 int in_merge_bases(struct commit *, struct commit **, int);
 
-extern int interactive_add(void);
+extern int interactive_add(const char *path);
 extern void add_files_to_cache(int verbose, const char *prefix, const char **files);
 extern int rerere(void);
 
-- 
1.5.3.5.737.gdee1b

[PATCH 4/4] Document optional file parameter to git-add--interactive

[Wincent Colaiuta]

Update the documentation for git-add to mention the new optional
file parameter to git-add--interactive.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 Documentation/git-add.txt |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 63829d9..4b44802 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -61,7 +61,10 @@ OPTIONS
 
 -i, \--interactive::
 	Add modified contents in the working tree interactively to
-	the index.
+	the index. If a single optional file argument is supplied the
+	initial command loop is bypassed and the 'patch' subcommand is
+	invoked for the specified file. See ``Interactive mode'' below
+	for details.
 
 -u::
 	Update only files that git already knows about. This is similar
-- 
1.5.3.5.737.gdee1b

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

[Jeff King]

On Wed, Nov 21, 2007 at 01:36:39PM +0100, Wincent Colaiuta wrote:

> If supplied a single file path parameter the git-add--interactive script
> now bypasses the command loop and jumps straight to the patch subcommand
> using the passed path. After returning from the subcommand the main
> command loop is entered. If a non-resolvable path is supplied the
> operation is a no-op and the command loop is entered.

Great, the lack of this feature has bugged me in the past. Thank you for
working on it. However...

> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index fb1e92a..8f21c03 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -803,6 +803,11 @@ sub main_loop {
>  	}
>  }
>  
> +die "add --interactive may take only 1 optional parameter" if ($#ARGV > 0);
>  refresh();
> +if ($#ARGV == 0) {
> +	patch_update_file($ARGV[0]);
> +}
>  status_cmd();
>  main_loop();
> +

Why only one file? How about something like this instead:

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..8036c95 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -804,5 +804,8 @@ sub main_loop {
 }
 
 refresh();
+foreach my $file (@ARGV) {
+	patch_update_file($file);
+}
 status_cmd();
 main_loop();


On top of that, it would be great to be able to do something like

  git-add -i *.c

and just get prompted for changed files (right now, you only get
prompted for changed files, but unchanged files seem to print a spurious
newline).

And at any rate, this would require fixing 3/4 to handle the multiple
files from git-add.

What do you think?

-Peff

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

[Wincent Colaiuta]

El 21/11/2007, a las 16:21, Jeff King escribió:

> On Wed, Nov 21, 2007 at 01:36:39PM +0100, Wincent Colaiuta wrote:
>
>> +if ($#ARGV == 0) {
>> +	patch_update_file($ARGV[0]);
>> +}
>
> Why only one file? How about something like this instead:
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index fb1e92a..8036c95 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -804,5 +804,8 @@ sub main_loop {
> }
>
> refresh();
> +foreach my $file (@ARGV) {
> +	patch_update_file($file);
> +}
> status_cmd();
> main_loop();

Yes, that makes sense. I wasn't sure how to handle multiple files, but  
I guess just looping through them is fine.

> On top of that, it would be great to be able to do something like
>
>  git-add -i *.c
>
> and just get prompted for changed files (right now, you only get
> prompted for changed files, but unchanged files seem to print a  
> spurious
> newline).

That spurious newline comes from the last line of the  
patch_update_file function (previously named patch_update_cmd):

	print "\n";

The solution will be to do an early return, printing nothing for those  
unchanged files (and same thing for untracked files). Will look into it.

> And at any rate, this would require fixing 3/4 to handle the multiple
> files from git-add.
>
> What do you think?

Should have some time later this evening or tomorrow to incorporate  
your suggestions.

Cheers,
Wincent

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ...
> On top of that, it would be great to be able to do something like
>
>   git-add -i *.c
>
> and just get prompted for changed files (right now, you only get
> prompted for changed files, but unchanged files seem to print a spurious
> newline).
>
> And at any rate, this would require fixing 3/4 to handle the multiple
> files from git-add.
>
> What do you think?

If we are to add path limited behaviour, I think it should also
grok "git-add -i sub/dir/".  IOW, you would want to have the
same path selection semantics as git-add without the
"interactive" bit.

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

[Wincent Colaiuta]

El 21/11/2007, a las 21:40, Junio C Hamano escribió:

> Jeff King <peff@peff.net> writes:
>
>> ...
>> On top of that, it would be great to be able to do something like
>>
>>  git-add -i *.c
>>
>> and just get prompted for changed files (right now, you only get
>> prompted for changed files, but unchanged files seem to print a  
>> spurious
>> newline).
>>
>> And at any rate, this would require fixing 3/4 to handle the multiple
>> files from git-add.
>>
>> What do you think?
>
> If we are to add path limited behaviour, I think it should also
> grok "git-add -i sub/dir/".  IOW, you would want to have the
> same path selection semantics as git-add without the
> "interactive" bit.

I can work on adding support for dirs as well as files, but am  
wondering what the desired behaviour is:

- Jeff would like to pass "*.c" and have it only add changed files,  
not unchanged or untracked files

- 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?

Of these two approaches, I suspect that the latter would be more  
"correct", because it would be consistent with the "not interactive"  
version of git-add.

Cheers,
Wincent

Updates: teaching git-add--interactive to accept a file param

[Wincent Colaiuta]

Here are some incremental updates incorporating the feedback received on the
patch series that I sent earlier today. These apply on top of the previous
patches; when this series matures I'll rebase and squash this down to a
smaller number of commits, but for the timebeing I think it's clearer if I
post these modifications separately so they can be reviewed in themselves.

The modifications are:

      Suppress spurious linefeeds in git-add--interactive
      Teach git-add--interactive to accept multiple file params
      Teach builtin-add to pass multiple paths to git-add--interactive
      Update git-add documentation for multiple interactive paths

Next step will be to follow-up on Junio's suggestion that we should support
directory paths, not just file paths, and replicate the semantics of
non-interactive git-add.

 Documentation/git-add.txt |    4 ++--
 builtin-add.c             |   23 +++++++++++++----------
 commit.h                  |    2 +-
 git-add--interactive.perl |    5 ++---
 4 files changed, 18 insertions(+), 16 deletions(-)

[PATCH 1/4] Suppress spurious linefeeds in git-add--interactive

[Wincent Colaiuta]

Eliminate the spurious linefeed echoed to the terminal when passing an
untracked or unchanged file as a parameter to git-add--interactive.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-add--interactive.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 8f21c03..95e537c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -336,6 +336,7 @@ sub add_untracked_cmd {
 sub parse_diff {
 	my ($path) = @_;
 	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
+	return undef if ($#diff == -1);
 	my (@hunk) = { TEXT => [] };
 
 	for (@diff) {
@@ -571,6 +572,7 @@ sub patch_update_file {
 	my ($ix, $num);
 	my $path = shift;
 	my ($head, @hunk) = parse_diff($path);
+	return unless $head;
 	for (@{$head->{TEXT}}) {
 		print;
 	}
-- 
1.5.3.6.867.g539b6-dirty

[PATCH 2/4] Teach git-add--interactive to accept multiple file params

[Wincent Colaiuta]

As per Jeff King's suggestion, this commit teaches git-add--interactive
to accept multiple optional file parameters, which will be fed into the
"patch" subcommand one by one. After all parameters are processed we
drop back into the standard command loop.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-add--interactive.perl |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 95e537c..2bba07d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -805,10 +805,9 @@ sub main_loop {
 	}
 }
 
-die "add --interactive may take only 1 optional parameter" if ($#ARGV > 0);
 refresh();
-if ($#ARGV == 0) {
-	patch_update_file($ARGV[0]);
+foreach my $file (@ARGV) {
+	patch_update_file($file);
 }
 status_cmd();
 main_loop();
-- 
1.5.3.6.867.g539b6-dirty

[PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive

[Wincent Colaiuta]

Instead of just accepting a single file parameter, git-add now accepts
any number of path parameters, fowarding them to git-add--interactive.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 builtin-add.c |   23 +++++++++++++----------
 commit.h      |    2 +-
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 278c02e..13f27e8 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -135,11 +135,17 @@ static void refresh(int verbose, const char **pathspec)
         free(seen);
 }
 
-int interactive_add(const char *path)
+int interactive_add(const char **argv, int argc)
 {
-	const char *argv[3] = { "add--interactive", path, NULL };
-
-	return run_command_v_opt(argv, RUN_GIT_CMD);
+	int status;
+	const char **args = xmalloc(sizeof(const char *) * (argc + 1));
+	args[0] = "add--interactive";
+	memcpy((void *)args + sizeof(const char *), argv, sizeof(const char *) * argc);
+	args[argc + 1] = NULL;
+
+	status = run_command_v_opt(args, RUN_GIT_CMD);
+	free(args);
+	return status;
 }
 
 static struct lock_file lock_file;
@@ -170,13 +176,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	argc = parse_options(argc, argv, builtin_add_options,
 			  builtin_add_usage, 0);
 	if (add_interactive) {
-		if (argc > 1)
-			die("add --interactive may take only 1 optional "
-			    "parameter");
-		else if (argc == 1)
-			exit(interactive_add(argv[0]));
+		if (argc > 0)
+			exit(interactive_add(argv, argc));
 		else
-			exit(interactive_add(NULL));
+			exit(interactive_add(NULL, 0));
 	}
 
 	git_config(git_default_config);
diff --git a/commit.h b/commit.h
index 03a6ec5..3a398fc 100644
--- a/commit.h
+++ b/commit.h
@@ -113,7 +113,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
 
 int in_merge_bases(struct commit *, struct commit **, int);
 
-extern int interactive_add(const char *path);
+extern int interactive_add(const char **argv, int argc);
 extern void add_files_to_cache(int verbose, const char *prefix, const char **files);
 extern int rerere(void);
 
-- 
1.5.3.6.867.g539b6-dirty

[PATCH 4/4] Update git-add documentation for multiple interactive paths

[Wincent Colaiuta]

Tweak the documentation to reflect that git-add--interactive can now
accept multiple path parameters.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 Documentation/git-add.txt |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index 4b44802..0675dbe 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -61,9 +61,9 @@ OPTIONS
 
 -i, \--interactive::
 	Add modified contents in the working tree interactively to
-	the index. If a single optional file argument is supplied the
+	the index. If optional path arguments are supplied the
 	initial command loop is bypassed and the 'patch' subcommand is
-	invoked for the specified file. See ``Interactive mode'' below
+	invoked for each specified path. See ``Interactive mode'' below
 	for details.
 
 -u::
-- 
1.5.3.6.867.g539b6-dirty

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

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> 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.

[PATCH] Add path-limiting to git-add--interactive

[Wincent Colaiuta]

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 <win@wincent.com>
---
 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

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

[Wincent Colaiuta]

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

> Wincent Colaiuta <win@wincent.com> 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.

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

Re: [PATCH 1/4] Suppress spurious linefeeds in git-add--interactive

[Jeff King]

On Thu, Nov 22, 2007 at 01:02:50AM +0100, Wincent Colaiuta wrote:

> +	return undef if ($#diff == -1);

Style nit: I think the rest of the code generally uses (and I prefer)
"@diff" to get the number of elements. So:

  return undef unless @diff;

or I might even have written

  my @diff = ...
    or return undef;

but perhaps I am the only one who finds $#array comparisons to -1 hard
on the eyes.

-Peff

Re: [PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive

[Jeff King]

On Thu, Nov 22, 2007 at 01:02:52AM +0100, Wincent Colaiuta wrote:

> -int interactive_add(const char *path)
> +int interactive_add(const char **argv, int argc)
>  {
> -	const char *argv[3] = { "add--interactive", path, NULL };
> -
> -	return run_command_v_opt(argv, RUN_GIT_CMD);
> +	int status;
> +	const char **args = xmalloc(sizeof(const char *) * (argc + 1));
> +	args[0] = "add--interactive";
> +	memcpy((void *)args + sizeof(const char *), argv, sizeof(const char *) * argc);

The source for the memcpy (argv) is sometimes NULL. The standard forbids
this, even when the size field is 0. I have no idea if any reasonable
implementations actually care.

But...

> +		if (argc > 0)
> +			exit(interactive_add(argv, argc));
>  		else
> -			exit(interactive_add(NULL));
> +			exit(interactive_add(NULL, 0));

There really is no reason to pass NULL at all (since the argc limits us
anyway), so we can just get rid of the conditional and simplify to:

  exit(interactive_add(argv, argc));

for both cases.

-Peff

Re: [PATCH] Add path-limiting to git-add--interactive

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> @@ -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;

Eek.  why?

Did you mean to say:

    my @tracked = run_cmd_pipe(gw(git ls-files --exclude-standard --) @ARGV);

It would also make sense to use --error-unmatch and perhaps --with-tree=HEAD
like git-commit.sh does.

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

[Jeff King]

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

Re: [PATCH] Add path-limiting to git-add--interactive

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> @@ -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;
>
> Eek.  why?
>
> Did you mean to say:
>
>     my @tracked = run_cmd_pipe(gw(git ls-files --exclude-standard --) @ARGV);
>
> It would also make sense to use --error-unmatch and perhaps --with-tree=HEAD
> like git-commit.sh does.

Actually, the ls-files need to be chomped, but I'd prefer to
run -z form of the command and split with NUL.  Does run_cmd_pipe()
crap^Wstuff support that?

On top of:

 * refactor patch_update_cmd,
 * teach builtin-add to pass multiple paths,
 * add path-limiting to git-add--interacgtive (with an obvious
   suggested above),

the attached patch teaches [p]atch subcommand to take multiple
selections.  With these, you can do:

	$ git add -i 'u*.h'
	What now> p
                   staged     unstaged path
          1:    unchanged        +1/-0 unpack-trees.h
          2:    unchanged        +1/-0 utf8.h
        Patch update>> *
        diff --git a/unpack-trees.h b/unpack-trees.h
	...
        Stage this hunk [y/n/a/d/?]? y
        ...
	diff --git a/utf8.h b/utf8.h
        ...

-- >8 --
git-add -i: allow multiple selection in [p]atch subcommand

This allows more than one files from the list to be chosen from
the patch subcommand instead of going through the file one by
one.

This also updates the "list-and-choose" UI for usability.  When
the prompt ends with ">>", if you type '*' to choose all
choices, the prompt immediately returns the choice without
requiring an extra empty line to confirm the selection.

---
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 9236ffc..372dc2c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -256,7 +256,7 @@ sub list_and_choose {
 				$chosen[$i] = $choose;
 			}
 		}
-		last if ($opts->{IMMEDIATE});
+		last if ($opts->{IMMEDIATE} || $line eq '*');
 	}
 	for ($i = 0; $i < @stuff; $i++) {
 		if ($chosen[$i]) {
@@ -563,12 +563,12 @@ sub patch_update_cmd {
 	@mods = grep { !($_->{BINARY}) } @mods;
 	return if (!@mods);
 
-	my ($it) = list_and_choose({ PROMPT => 'Patch update',
-				     SINGLETON => 1,
-				     IMMEDIATE => 1,
-				     HEADER => $status_head, },
-				   @mods);
-	patch_update_file($it->{VALUE}) if ($it);
+	my (@them) = list_and_choose({ PROMPT => 'Patch update',
+				       HEADER => $status_head, },
+				     @mods);
+	for (@them) {
+		patch_update_file($_->{VALUE});
+	}
 }
 
 sub patch_update_file {

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> 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?

I am moderately negative about "paths imply jump to patch
subcommand".  An option to git-add--interactive that tells which
subcommand to initially choose is probably acceptable, though.

Would the patch I just sent out (you need to assemble the parts)
make things easier?

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

[Jeff King]

On Thu, Nov 22, 2007 at 01:50:31AM -0800, Junio C Hamano wrote:

> I am moderately negative about "paths imply jump to patch
> subcommand".  An option to git-add--interactive that tells which
> subcommand to initially choose is probably acceptable, though.

I agree that an option is nicer; it's just that it may be awkward since
'-i' is itself already an option.

> Would the patch I just sent out (you need to assemble the parts)
> make things easier?

Perhaps. This feature doesn't come up all that often for me, but I will
try running with your patch for a while to see if I like it.

-Peff

Re: [PATCH 1/4] Suppress spurious linefeeds in git-add--interactive

[Wincent Colaiuta]

El 22/11/2007, a las 9:59, Jeff King escribió:

> On Thu, Nov 22, 2007 at 01:02:50AM +0100, Wincent Colaiuta wrote:
>
>> +	return undef if ($#diff == -1);
>
> Style nit: I think the rest of the code generally uses (and I prefer)
> "@diff" to get the number of elements. So:
>
>  return undef unless @diff;
>
> or I might even have written
>
>  my @diff = ...
>    or return undef;
>
> but perhaps I am the only one who finds $#array comparisons to -1 hard
> on the eyes.

No, I agree with you; it does read nicer. I'm not a real Perl  
programmer, so everything I do I have to do it with a copy of  
"Programming Perl" next to me to find out how to do things. It's handy  
to have advice from people more familiar with the idioms. Will  
incorporate something like this into the series:

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index a5a07bc..0a8f0a9 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -60,7 +60,7 @@ sub list_modified {
  		defined run_cmd_pipe(qw(git ls-files
  			                --exclude-standard --), $_)
  	} @ARGV;
-	return if $#tracked == -1 && $#ARGV != -1;
+	return if !@tracked && @ARGV;

  	for (run_cmd_pipe(qw(git diff-index --cached
  			     --numstat --summary HEAD --), @tracked)) {
@@ -340,8 +340,8 @@ sub add_untracked_cmd {

  sub parse_diff {
  	my ($path) = @_;
-	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path);
-	return undef if ($#diff == -1);
+	my @diff = run_cmd_pipe(qw(git diff-files -p --), $path)
+	    or return undef;
  	my (@hunk) = { TEXT => [] };

  	for (@diff) {



Cheers,
Wincent

Re: [PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive

[Wincent Colaiuta]

El 22/11/2007, a las 10:08, Jeff King escribió:

> On Thu, Nov 22, 2007 at 01:02:52AM +0100, Wincent Colaiuta wrote:
>
>> -int interactive_add(const char *path)
>> +int interactive_add(const char **argv, int argc)
>> {
>> -	const char *argv[3] = { "add--interactive", path, NULL };
>> -
>> -	return run_command_v_opt(argv, RUN_GIT_CMD);
>> +	int status;
>> +	const char **args = xmalloc(sizeof(const char *) * (argc + 1));
>> +	args[0] = "add--interactive";
>> +	memcpy((void *)args + sizeof(const char *), argv, sizeof(const  
>> char *) * argc);
>
> The source for the memcpy (argv) is sometimes NULL. The standard  
> forbids
> this, even when the size field is 0. I have no idea if any reasonable
> implementations actually care.

Good point. I've now conditionalized the memcpy with an "if (argc >  
0)". While I was at it I also got rid of the unneeded cast to void *.

> But...
>
>> +		if (argc > 0)
>> +			exit(interactive_add(argv, argc));
>> 		else
>> -			exit(interactive_add(NULL));
>> +			exit(interactive_add(NULL, 0));
>
> There really is no reason to pass NULL at all (since the argc limits  
> us
> anyway), so we can just get rid of the conditional and simplify to:
>
>  exit(interactive_add(argv, argc));
>
> for both cases.

Yes, that's a nice cleanup. Thanks for suggesting it.

Cheers,
Wincent

Re: [PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive

[Jeff King]

On Thu, Nov 22, 2007 at 11:28:17AM +0100, Wincent Colaiuta wrote:

>>> +	memcpy((void *)args + sizeof(const char *), argv, sizeof(const char *) * 
>>> argc);
>>
>> The source for the memcpy (argv) is sometimes NULL. The standard forbids
>> this, even when the size field is 0. I have no idea if any reasonable
>> implementations actually care.
>
> Good point. I've now conditionalized the memcpy with an "if (argc > 0)". 
> While I was at it I also got rid of the unneeded cast to void *.

I don't think you need it if you do the other cleanup (since you will
always be passing a valid argv pointer).

-Peff

Re: [PATCH 1/4] Suppress spurious linefeeds in git-add--interactive

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> No, I agree with you; it does read nicer. I'm not a real Perl  
> programmer, so everything I do I have to do it with a copy of  
> "Programming Perl" next to me to find out how to do things. It's handy  
> to have advice from people more familiar with the idioms. Will  
> incorporate something like this into the series:

I took the liberty of rewriting them and will push out on pu
shortly hopefully before I go to bed.

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

[Wincent Colaiuta]

El 22/11/2007, a las 10:50, Junio C Hamano escribió:

> Jeff King <peff@peff.net> writes:
>
>> 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?
>
> I am moderately negative about "paths imply jump to patch
> subcommand".  An option to git-add--interactive that tells which
> subcommand to initially choose is probably acceptable, though.

Let me explain a little why I started this series. Basically, given  
file "foo" which had multiple changes in it I wanted to stage only  
selected hunks; so I typed:

	git add -i foo

Which of course didn't work. What I wanted to do was jump straight  
into the patch subcommand (hence this series).

If I wanted to add entire files I would have just typed:

	git add foo

So I don't think the proposal to add "-p" (jump to "patch" subcommand)  
and "-a" (jump to "add untracked" subcommand) are a very good idea,  
seeing as we already have builtin-add for adding entire files.

Cheers,
Wincent

Re: [PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive

[Wincent Colaiuta]

El 22/11/2007, a las 11:33, Jeff King escribió:

> On Thu, Nov 22, 2007 at 11:28:17AM +0100, Wincent Colaiuta wrote:
>
>>>> +	memcpy((void *)args + sizeof(const char *), argv, sizeof(const  
>>>> char *) *
>>>> argc);
>>>
>>> The source for the memcpy (argv) is sometimes NULL. The standard  
>>> forbids
>>> this, even when the size field is 0. I have no idea if any  
>>> reasonable
>>> implementations actually care.
>>
>> Good point. I've now conditionalized the memcpy with an "if (argc >  
>> 0)".
>> While I was at it I also got rid of the unneeded cast to void *.
>
> I don't think you need it if you do the other cleanup (since you will
> always be passing a valid argv pointer).

True, argv will never be NULL. We'll still be doing a zero-byte memcpy  
though, which I guess is not a big deal here. I'll drop the conditional.

Cheers,
Wincent

Re: [PATCH] Add path-limiting to git-add--interactive

[Wincent Colaiuta]

El 22/11/2007, a las 10:13, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> @@ -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;
>
> Eek.  why?
>
> Did you mean to say:
>
>    my @tracked = run_cmd_pipe(gw(git ls-files --exclude-standard --)  
> @ARGV);

Bah, indeed that will work. I mistakenly (stupidly?) thought I had to  
check each path in @ARGV one at a time; didn't realize that I could  
pass in all at once and that it would do the right thing.... I've  
incorporated the suggested change.

> It would also make sense to use --error-unmatch and perhaps --with- 
> tree=HEAD
> like git-commit.sh does.

My reading of the run_cmd_pipe function indicates that those options  
won't have any effect at all because run_cmd_pipe doesn't check the  
exit status of the command.

Cheers,
Wincent

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

[Jeff King]

On Thu, Nov 22, 2007 at 11:44:54AM +0100, Wincent Colaiuta wrote:

> So I don't think the proposal to add "-p" (jump to "patch" subcommand) and 
> "-a" (jump to "add untracked" subcommand) are a very good idea, seeing as we 
> already have builtin-add for adding entire files.

I agree that "git-add -i untracked_file" is a bit useless. I think it
might be more useful if you could split the one giant hunk (which
git-add--interactive does not currently allow) into smaller hunks, and
then just add parts of them (so conceptually treat it like it had been a
tracked file with empty contents, and go to the patch menu). Of course,
there is currently no way to usefully split a hunk that has consecutive
added lines, so that would have to be figured out, too.

So I think it is not so much "this other thing is useful right now" as
"do we want to paint ourselves in a corner for making it (or other
things) useful later on?" And assuming that having arguments means
"start the patch menu on these files" doesn't leave room for changes
later.

-Peff

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

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> If I wanted to add entire files I would have just typed:
>
> 	git add foo
>
> So I don't think the proposal to add "-p" (jump to "patch" subcommand)  
> and "-a" (jump to "add untracked" subcommand) are a very good idea,  
> seeing as we already have builtin-add for adding entire files.

It is quite valid for users to run:

	git add -i \*.sh

and be able to choose from a list which paths to stage (as a
whole), as well as choose from a list which files to run the
per-hunk staging interface.  "git add \*.sh" won't give you any
chance to choose which ones to stage, and that's what we have
"-i" (interactive) mode in "git add" for.

I think you can massage "git add --partial foo" given by the
user internally into "git-add--interactive -i --patch foo".  I
strongly suspect that "direct to patch subcommand" mode needs
more than just initially jumping into the subcommand (for
example, you would want to exit when the patch selection
interaction ends, without going back to the main menu), and we
would want a signal stronger than mere presense of pathspecs to
trigger such a specialized behaviour.

By the way, the arguments on the command line to git commands
after "--" are generally pathspecs, iow, patterns to specify
groups of files.  Please do not introduce unnecessary
inconsistencies to the UI by requiring them to be exact pathname
only in this particular mode of the command and nowhere else.

There was one funny thing I fixed up.  The arguments to the
interactive_add() function in builtin-add.c was like this:

	int interactive_add(const char **argv, int argc)

Anybody who writes a function with such a signature and do not
notice its craziness before sending it out either (1) has never
programmed in C, (2) did not review the code before submitting,
or (3) worked too hard and was too tired.

I suspect, judging from the timestamp of your message, it was
(3) this time.  The collaborative development is not a race ---
don't work too hastily and too hard; please relax and review
after a good night's sleep before sending things out.

Re: [PATCH] Add path-limiting to git-add--interactive

[Wincent Colaiuta]

El 22/11/2007, a las 10:45, Junio C Hamano escribió:

> the attached patch teaches [p]atch subcommand to take multiple
> selections.  With these, you can do:
>
> 	$ git add -i 'u*.h'
> 	What now> p
>                   staged     unstaged path
>          1:    unchanged        +1/-0 unpack-trees.h
>          2:    unchanged        +1/-0 utf8.h
>        Patch update>> *
>        diff --git a/unpack-trees.h b/unpack-trees.h
> 	...
>        Stage this hunk [y/n/a/d/?]? y
>        ...
> 	diff --git a/utf8.h b/utf8.h
>        ...
>
> -- >8 --

Nice usability improvement.

Cheers,
Wincent

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

[Jeff King]

On Thu, Nov 22, 2007 at 03:29:03AM -0800, Junio C Hamano wrote:

> There was one funny thing I fixed up.  The arguments to the
> interactive_add() function in builtin-add.c was like this:
> 
> 	int interactive_add(const char **argv, int argc)
> 
> Anybody who writes a function with such a signature and do not
> notice its craziness before sending it out either (1) has never
> programmed in C, (2) did not review the code before submitting,
> or (3) worked too hard and was too tired.

Heh. I looked the patch over, noticed a subtle memcpy issue, and didn't
even notice the craziness. I claim 3, also.

-Peff

Re: [PATCH 3/4] Teach builtin-add to pass multiple paths to git-add--interactive

[Jeff King]

On Thu, Nov 22, 2007 at 12:02:17PM +0100, Wincent Colaiuta wrote:

> True, argv will never be NULL. We'll still be doing a zero-byte memcpy 
> though, which I guess is not a big deal here. I'll drop the conditional.

Right. And zero-byte memcpy _is_ allowed by the standard, so it will
already internally have some form of that conditional.

-Peff

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

[Wincent Colaiuta]

El 22/11/2007, a las 12:29, Junio C Hamano escribió:

> It is quite valid for users to run:
>
> 	git add -i \*.sh
>
> and be able to choose from a list which paths to stage (as a
> whole), as well as choose from a list which files to run the
> per-hunk staging interface.  "git add \*.sh" won't give you any
> chance to choose which ones to stage, and that's what we have
> "-i" (interactive) mode in "git add" for.
>
> I think you can massage "git add --partial foo" given by the
> user internally into "git-add--interactive -i --patch foo".  I
> strongly suspect that "direct to patch subcommand" mode needs
> more than just initially jumping into the subcommand (for
> example, you would want to exit when the patch selection
> interaction ends, without going back to the main menu)

This will become especially important if we decide to allow "git  
commit --interactive" to accept a path spec as well (although exactly  
when we should exit isn't immediately clear to me); I didn't touch  
this for the time-being because builtin-commit is getting very close  
to replacing git-commit.sh.

> and we
> would want a signal stronger than mere presense of pathspecs to
> trigger such a specialized behaviour.
>
> By the way, the arguments on the command line to git commands
> after "--" are generally pathspecs, iow, patterns to specify
> groups of files.  Please do not introduce unnecessary
> inconsistencies to the UI by requiring them to be exact pathname
> only in this particular mode of the command and nowhere else.

Well, I it wasn't my intention to introduce any such requirement. The  
path parameters get passed in and eventually handed over unmodified to:

	git diff-files -p --

Which was what was previously in the parse_diff function. It turns out  
that if you pass something like \*.sh to git-diff-files then it won't  
match, but it's not a restriction that I chose to impose, rather it's  
a consequence of the way git-diff-files works. Are there options we  
could pass to avoid that restriction?

In any case, one good thing to come out of this discussion is the  
ability to use pathspecs to limit the number of files displayed when  
in interactive mode. But it wasn't what I was originally aiming for.  
My initial goal was to have git-add--interactive do something useful  
and convenient when you type:

	git add -i foo

If we don't jump straight to the subcommand then the user has to:

	- press 5 or p, then Enter
	- press 1, then Enter

This was easier than it was before (same number of keystrokes, but  
fewer files to visually scan in the list of candidates), but it's  
still not that much easier.

Or if we add some kind of "--patch" switch, instead do:

	git add -i --patch foo

Once again, not that easy but you could always set up a Git alias, I  
guess, as a shortcut. Not sure whether this is offers enough of a  
workflow improvement to make it worthwhile (ie. you may as well just  
fire up git-gui).

> There was one funny thing I fixed up.  The arguments to the
> interactive_add() function in builtin-add.c was like this:
>
> 	int interactive_add(const char **argv, int argc)
>
> Anybody who writes a function with such a signature and do not
> notice its craziness before sending it out either (1) has never
> programmed in C, (2) did not review the code before submitting,
> or (3) worked too hard and was too tired.
>
> I suspect, judging from the timestamp of your message, it was
> (3) this time.  The collaborative development is not a race ---
> don't work too hastily and too hard; please relax and review
> after a good night's sleep before sending things out.

In any case I've rebased and squished the changes down to a nice 4- 
patch series, incorporating all the feedback given so far. Will sit on  
it a while before sending it out, as you suggest. But I don't really  
have a clear idea where to go forward from here.

Cheers,
Wincent

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

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> El 22/11/2007, a las 12:29, Junio C Hamano escribió:
>
>> By the way, the arguments on the command line to git commands
>> after "--" are generally pathspecs, iow, patterns to specify
>> groups of files.  Please do not introduce unnecessary
>> inconsistencies to the UI by requiring them to be exact pathname
>> only in this particular mode of the command and nowhere else.
>
> Well, I it wasn't my intention to introduce any such requirement. The
> path parameters get passed in and eventually handed over unmodified to:
>
> 	git diff-files -p --

What I was referring to was this in your original:

>> +	my @tracked = grep {
>> +		defined run_cmd_pipe(qw(git ls-files
>> +			                --exclude-standard --), $_)
>> +	} @ARGV;

It picks elements from @ARGV that ls-files says "Ok" back, not
the expanded result from ls-files, and @tracked elements are
used later as filenames.  Which means you are expecting @ARGV to
contain only concrete filenames, not pathspec.

> Or if we add some kind of "--patch" switch, instead do:
>
> 	git add -i --patch foo

You do not need both "-i" and "--patch", do you?  Plain git-add
does not take --patch anyway so it can pass --patch to the
underlying interactive one.

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

[Wincent Colaiuta]

El 22/11/2007, a las 20:29, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> Or if we add some kind of "--patch" switch, instead do:
>>
>> 	git add -i --patch foo
>
> You do not need both "-i" and "--patch", do you?  Plain git-add
> does not take --patch anyway so it can pass --patch to the
> underlying interactive one.

Good point.

Cheers,
Wincent