Re: [PATCH] Highlight keyboard shortcuts in git-add--interactive

Wincent Colaiuta [off-list ref] writes:

I like the general idea of making it more obvious that you can use the
unique prefix, but I think you should make list_and_choose do this
automatically without adding a redundant element in the command array.

If you do so, the same highlighting will automatically appear when you
are picking which paths to update in the update subcommand, for example.

El 29/11/2007, a las 0:56, Junio C Hamano escribió:

Wincent Colaiuta [off-list ref] writes:

quoted

@@ -774,14 +774,14 @@ EOF

}

sub main_loop {
-	my @cmd = ([ 'status', \&status_cmd, ],
-		   [ 'update', \&update_cmd, ],
-		   [ 'revert', \&revert_cmd, ],
-		   [ 'add untracked', \&add_untracked_cmd, ],
-		   [ 'patch', \&patch_update_cmd, ],
-		   [ 'diff', \&diff_cmd, ],
-		   [ 'quit', \&quit_cmd, ],
-		   [ 'help', \&help_cmd, ],
+	my @cmd = ([ 'status', \&status_cmd, '[s]tatus', ],
+		   [ 'update', \&update_cmd, '[u]date', ],
+		   [ 'revert', \&revert_cmd, '[r]evert', ],
+		   [ 'add untracked', \&add_untracked_cmd, '[a]dd untracked', ],
+		   [ 'patch', \&patch_update_cmd, '[p]atch', ],
+		   [ 'diff', \&diff_cmd, '[d]iff', ],
+		   [ 'quit', \&quit_cmd, '[q]uit', ],
+		   [ 'help', \&help_cmd, '[h]elp', ],
	);

I like the general idea of making it more obvious that you can use the
unique prefix, but I think you should make list_and_choose do this
automatically without adding a redundant element in the command array.

If you do so, the same highlighting will automatically appear when you
are picking which paths to update in the update subcommand, for  
example.

Yes, I did consider that, and it's very easy when all the options have  
a unique, single-letter prefix, as is the case with the main command  
loop. But what to do if you've got a bunch of paths with lengthy  
common prefixes? eg. what would you highlight here?

lib/ssl/crypto/foo.c
lib/ssl/crypto/bar.c
lib/ssl/crypto/baz.c

Highlighting "lib/ssl/crypto/f", "lib/ssl/crypto/bar" and "lib/ssl/ 
crypto/baz" doesn't sound like much help... Maybe there should be some  
limit: if you need to go more than 3 characters deep in order to  
differentiate unique prefixes then perhaps highlighting should be  
omitted in that case. What do you think of that idea?

Cheers,
Wincent

The user interface provided by the command loop in git-add--interactive
gives the impression that subcommands can only be launched by entering
an integer identifier from 1 through 8.

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the first matching option.

This patch makes this feature a little more obvious by highlighting the
first character of each subcommand (for example "patch" is displayed as
"[p]atch").

A new function is added to detect the shortest unique prefix and this
is used to decide what to highlight. Highlighting is also applied when
choosing files.

In the case where the common prefix may be unreasonably large
highlighting is omitted; in this patch the soft limit (above which the
highlighting will be omitted for a particular item) is 0 (in other words,
there is no soft limit) and the hard limit (above which highlighting will
be omitted for all items) is 3, but this can be tweaked.

The actual highlighting is done by the highlight_prefix function, which
will enable us to implement ANSI color code-based highlighting (most
likely using underline or boldface) in the future.

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

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..6e5781b 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -44,7 +44,6 @@ my $status_fmt = '%12s %12s %s';
 my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
 
 # Returns list of hashes, contents of each of which are:
-# PRINT:	print message
 # VALUE:	pathname
 # BINARY:	is a binary path
 # INDEX:	is index different from HEAD?

@@ -122,8 +121,6 @@ sub list_modified {
 		}
 		push @return, +{
 			VALUE => $_,
-			PRINT => (sprintf $status_fmt,
-				  $it->{INDEX}, $it->{FILE}, $_),
 			%$it,
 		};
 	}

@@ -159,10 +156,82 @@ sub find_unique {
 	return $found;
 }
 
+# inserts string into trie and updates count for each character
+sub update_trie {
+	my ($trie, $string) = @_;
+	foreach (split //, $string) {
+		$trie = $trie->{$_} ||= {COUNT => 0};
+		$trie->{COUNT}++;
+	}
+}
+
+# returns an array of tuples (prefix, remainder)
+sub find_unique_prefixes {
+	my @stuff = @_;
+	my @return = ();
+
+	# any single prefix exceeding the soft limit is omitted
+	# if any prefix exceeds the hard limit all are omitted
+	# 0 indicates no limit
+	my $soft_limit = 0;
+	my $hard_limit = 3;
+
+	# build a trie modelling all possible options
+	my %trie;
+	foreach my $print (@stuff) {
+		if ((ref $print) eq 'ARRAY') {
+			$print = $print->[0];
+		}
+		else {
+			$print = $print->{VALUE};
+		}
+		update_trie(\%trie, $print);
+		push @return, $print;
+	}
+
+	# use the trie to find the unique prefixes
+	for (my $i = 0; $i < @return; $i++) {
+		my $ret = $return[$i];
+		my @letters = split //, $ret;
+		my %search = %trie;
+		my ($prefix, $remainder);
+		my $j;
+		for ($j = 0; $j < @letters; $j++) {
+			my $letter = $letters[$j];
+			if ($search{$letter}{COUNT} == 1) {
+				$prefix = substr $ret, 0, $j + 1;
+				$remainder = substr $ret, $j + 1;
+				last;
+			}
+			else {
+				my $prefix = substr $ret, 0, $j;
+				return ()
+				    if ($hard_limit && $j + 1 > $hard_limit);
+			}
+			%search = %{$search{$letter}};
+		}
+		if ($soft_limit && $j + 1 > $soft_limit) {
+			$prefix = undef;
+			$remainder = $ret;
+		}
+		$return[$i] = [$prefix, $remainder];
+	}
+	return @return;
+}
+
+# given a prefix/remainder tuple return a string with the prefix highlighted
+# for now use square brackets; later might use ANSI colors (underline, bold)
+sub highlight_prefix {
+	my $prefix = shift;
+	my $remainder = shift;
+	$prefix ? "[$prefix]$remainder" : $remainder;
+}
+
 sub list_and_choose {
 	my ($opts, @stuff) = @_;
 	my (@chosen, @return);
 	my $i;
+	my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
 
       TOPLOOP:
 	while (1) {

@@ -179,10 +248,18 @@ sub list_and_choose {
 			my $print = $stuff[$i];
 			if (ref $print) {
 				if ((ref $print) eq 'ARRAY') {
-					$print = $print->[0];
+					$print = @prefixes ?
+					    highlight_prefix(@{$prefixes[$i]}) :
+					    $print->[0];
 				}
 				else {
-					$print = $print->{PRINT};
+					my $value = @prefixes ?
+					    highlight_prefix(@{$prefixes[$i]}) :
+					    $print->{VALUE};
+					$print = sprintf($status_fmt,
+					    $print->{INDEX},
+					    $print->{FILE},
+					    $value);
 				}
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);

-- 
1.5.3.6.953.gdffc

On Thu, Nov 29, 2007 at 01:00:38PM +0100, Wincent Colaiuta wrote:

A new function is added to detect the shortest unique prefix and this
is used to decide what to highlight. Highlighting is also applied when
choosing files.

I think this is very nicely implemented.

Acked-by: Jeff King <redacted>

+# returns an array of tuples (prefix, remainder)
+sub find_unique_prefixes {
+	my @stuff = @_;

I know we generally use this more C-ish argument convention to document
"here are the arguments to this function", but it does actually make a
copy of the @_ array (and using @_ implies a potentially large number of
arguments).

It probably doesn't matter here, though, since add--interactive is not
performance critical, and you probably can't have more than a few dozen
entries before it becomes unreadable anyway.

-Peff

Wincent Colaiuta [off-list ref] writes:

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the first matching option.

I'd run s/the first/the uniquely/ here.

When list_and_choose() function is letting you choose more than one
items, its prompt becomes ">> ", instead of "> " that is used for a
singleton choice.  To that prompt, you can say "3-7" (Add these 5 items
to the choice), "*" (I want all of them), "-2-4" (exclude 2 and 3 and 4
from the set I have chosen so far).  These are also "hidden", and need
to be documented, but that would be a separate patch.

+# given a prefix/remainder tuple return a string with the prefix highlighted
+# for now use square brackets; later might use ANSI colors (underline, bold)
+sub highlight_prefix {
+	my $prefix = shift;
+	my $remainder = shift;
+	$prefix ? "[$prefix]$remainder" : $remainder;
+}

I'd rewrite the last line to:

	return (defined $prefix) ? "[$prefix]$remainder" : $remainder;

just in case the unique prefix is "0".  Otherwise you would lose the
first letter from "00ReadMe" and show remainder "0ReadMe" alone.

El 1/12/2007, a las 3:36, Junio C Hamano escribió:

Wincent Colaiuta [off-list ref] writes:

quoted

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the first matching option.

I'd run s/the first/the uniquely/ here.

When list_and_choose() function is letting you choose more than one
items, its prompt becomes ">> ", instead of "> " that is used for a
singleton choice.  To that prompt, you can say "3-7" (Add these 5  
items
to the choice), "*" (I want all of them), "-2-4" (exclude 2 and 3  
and 4
from the set I have chosen so far).  These are also "hidden", and need
to be documented, but that would be a separate patch.

Agreed that it belongs in a separate patch.

But I'm glad you brought this up as it reminds me of the need to watch  
out for those characters which have special meaning for  
list_and_choose().

I'd rewrite the last line to:

	return (defined $prefix) ? "[$prefix]$remainder" : $remainder;

just in case the unique prefix is "0".  Otherwise you would lose the
first letter from "00ReadMe" and show remainder "0ReadMe" alone.

Excellent catch. Crazy old perl; I didn't realize that "0" (the  
string, not the number) would evaluate to false.

Will send a separate mail with a revised, squashed patch with these  
changes:

- "s/the first/the uniquely/" in the commit message as you suggest

- filter out prefixes which contain characters with special meaning  
for list_and_choose()

- check "defined $prefix" rather than just "$prefix"

- also fixes a problem discovered while playing with this; it didn't  
play nicely with untracked files

Cheers,
Wincent

The user interface provided by the command loop in git-add--interactive
gives the impression that subcommands can only be launched by entering
an integer identifier from 1 through 8.

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the uniquely matching option.

This patch makes this feature a little more obvious by highlighting the
first character of each subcommand (for example "patch" is displayed as
"[p]atch").

A new function is added to detect the shortest unique prefix and this
is used to decide what to highlight. Highlighting is also applied when
choosing files.

In the case where the common prefix may be unreasonably large
highlighting is omitted; in this patch the soft limit (above which the
highlighting will be omitted for a particular item) is 0 (in other words,
there is no soft limit) and the hard limit (above which highlighting will
be omitted for all items) is 3, but this can be tweaked.

The actual highlighting is done by the highlight_prefix function, which
will enable us to implement ANSI color code-based highlighting (most
likely using underline or boldface) in the future.

Signed-off-by: Wincent Colaiuta <redacted>
---

Three things to note:

1. I don't actually find the "[p]atch" highlighting using brackets all
that attractive, especially when highlighting paths. This is
especially true when the common prefix is lengthy. This is why I've
set the "hard limit" to a very low 3 in this patch. I am basically
waiting on the stalled "color" series; once that's in "next" then I'd
like to switch to a highlight style that uses underlining.

2. The follow-up patch tweaks this so that the "Add untracked"
subcommand can benefit from it as well. Junio, you might want to squash
the two patches into one seeing as the second patch just refactors
something in the first patch; but I wanted to send them to the list as
two separate patches, at least for the purposes of review, because they
really are two separate behaviours.

3. I tried to find the patch that Junio sent out earlier allowing
multiple selection in the "Patch" subcommand, to see how this interplays
with that, but I couldn't locate it.

 git-add--interactive.perl |   97 ++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..0fb808f 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -44,7 +44,6 @@ my $status_fmt = '%12s %12s %s';
 my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
 
 # Returns list of hashes, contents of each of which are:
-# PRINT:	print message
 # VALUE:	pathname
 # BINARY:	is a binary path
 # INDEX:	is index different from HEAD?

@@ -122,8 +121,6 @@ sub list_modified {
 		}
 		push @return, +{
 			VALUE => $_,
-			PRINT => (sprintf $status_fmt,
-				  $it->{INDEX}, $it->{FILE}, $_),
 			%$it,
 		};
 	}

@@ -159,10 +156,92 @@ sub find_unique {
 	return $found;
 }
 
+# inserts string into trie and updates count for each character
+sub update_trie {
+	my ($trie, $string) = @_;
+	foreach (split //, $string) {
+		$trie = $trie->{$_} ||= {COUNT => 0};
+		$trie->{COUNT}++;
+	}
+}
+
+# returns an array of tuples (prefix, remainder)
+sub find_unique_prefixes {
+	my @stuff = @_;
+	my @return = ();
+
+	# any single prefix exceeding the soft limit is omitted
+	# if any prefix exceeds the hard limit all are omitted
+	# 0 indicates no limit
+	my $soft_limit = 0;
+	my $hard_limit = 3;
+
+	# build a trie modelling all possible options
+	my %trie;
+	foreach my $print (@stuff) {
+		if ((ref $print) eq 'ARRAY') {
+			$print = $print->[0];
+		}
+		elsif ((ref $print) eq 'HASH') {
+			$print = $print->{VALUE};
+		}
+		update_trie(\%trie, $print);
+		push @return, $print;
+	}
+
+	# use the trie to find the unique prefixes
+	for (my $i = 0; $i < @return; $i++) {
+		my $ret = $return[$i];
+		my @letters = split //, $ret;
+		my %search = %trie;
+		my ($prefix, $remainder);
+		my $j;
+		for ($j = 0; $j < @letters; $j++) {
+			my $letter = $letters[$j];
+			if ($search{$letter}{COUNT} == 1) {
+				$prefix = substr $ret, 0, $j + 1;
+				$remainder = substr $ret, $j + 1;
+				last;
+			}
+			else {
+				my $prefix = substr $ret, 0, $j;
+				return ()
+				    if ($hard_limit && $j + 1 > $hard_limit);
+			}
+			%search = %{$search{$letter}};
+		}
+		if ($soft_limit && $j + 1 > $soft_limit) {
+			$prefix = undef;
+			$remainder = $ret;
+		}
+		$return[$i] = [$prefix, $remainder];
+	}
+	return @return;
+}
+
+# filters out prefixes which have special meaning to list_and_choose()
+sub is_valid_prefix {
+	my $prefix = shift;
+	my $valid = (defined $prefix) &&
+	    !($prefix =~ /[\s,]/) && # separators
+	    !($prefix =~ /^-/) &&    # deselection
+	    !($prefix =~ /^\d+/) &&  # selection
+	    ($prefix ne '*');        # "all" wildcard
+}
+
+# given a prefix/remainder tuple return a string with the prefix highlighted
+# for now use square brackets; later might use ANSI colors (underline, bold)
+sub highlight_prefix {
+	my $prefix = shift;
+	my $remainder = shift;
+	is_valid_prefix($prefix) ? "[$prefix]$remainder" : $remainder;
+}
+
 sub list_and_choose {
 	my ($opts, @stuff) = @_;
 	my (@chosen, @return);
 	my $i;
+	my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
 
       TOPLOOP:
 	while (1) {

@@ -179,10 +258,18 @@ sub list_and_choose {
 			my $print = $stuff[$i];
 			if (ref $print) {
 				if ((ref $print) eq 'ARRAY') {
-					$print = $print->[0];
+					$print = @prefixes ?
+					    highlight_prefix(@{$prefixes[$i]}) :
+					    $print->[0];
 				}
 				else {
-					$print = $print->{PRINT};
+					my $value = @prefixes ?
+					    highlight_prefix(@{$prefixes[$i]}) :
+					    $print->{VALUE};
+					$print = sprintf($status_fmt,
+					    $print->{INDEX},
+					    $print->{FILE},
+					    $value);
 				}
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);

-- 
1.5.3.6.953.gdffc

Tweak the list_and_choose function so that untracked files will
use the automatic prefix detection machinery.

This works because while previously we handled arrays (command
menus), hashes (patch subcommand) now we explicitly handle strings
(add untracked subcommand).

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

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 0fb808f..a1aee21 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -256,21 +256,21 @@ sub list_and_choose {
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
 			my $print = $stuff[$i];
-			if (ref $print) {
-				if ((ref $print) eq 'ARRAY') {
-					$print = @prefixes ?
-					    highlight_prefix(@{$prefixes[$i]}) :
-					    $print->[0];
-				}
-				else {
-					my $value = @prefixes ?
-					    highlight_prefix(@{$prefixes[$i]}) :
-					    $print->{VALUE};
-					$print = sprintf($status_fmt,
-					    $print->{INDEX},
-					    $print->{FILE},
-					    $value);
-				}
+			my $ref = ref $print;
+			my $highlighted = highlight_prefix(@{$prefixes[$i]})
+			    if @prefixes;
+			if ($ref eq 'ARRAY') {
+				$print = $highlighted || $print->[0];
+			}
+			elsif ($ref eq 'HASH') {
+				my $value = $highlighted || $print->{VALUE};
+				$print = sprintf($status_fmt,
+				    $print->{INDEX},
+				    $print->{FILE},
+				    $value);
+			}
+			else {
+				$print = $highlighted || $print;
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);
 			if (($opts->{LIST_FLAT}) &&

-- 
1.5.3.6.953.gdffc

El 1/12/2007, a las 15:07, Wincent Colaiuta escribió:

+# filters out prefixes which have special meaning to  
list_and_choose()
+sub is_valid_prefix {
+	my $prefix = shift;
+	my $valid = (defined $prefix) &&
+	    !($prefix =~ /[\s,]/) && # separators
+	    !($prefix =~ /^-/) &&    # deselection
+	    !($prefix =~ /^\d+/) &&  # selection
+	    ($prefix ne '*');        # "all" wildcard
+}

Doh, that's supposed to be:

	return (defined $prefix)...

Not:

	my $valid = (defined $prefix)...

It actually works as is, but I had changed the "return" while working  
on the patch (for debugging) and forgot to change it back afterwards.

And yes, I did proofread the patch before sending it. I just didn't  
notice the first time around.

Cheers,
Wincent

The user interface provided by the command loop in git-add--interactive
gives the impression that subcommands can only be launched by entering
an integer identifier from 1 through 8.

A "hidden" feature is that any string can be entered, and an anchored
regex search is used to find the uniquely matching option.

This patch makes this feature a little more obvious by highlighting the
first character of each subcommand (for example "patch" is displayed as
"[p]atch").

A new function is added to detect the shortest unique prefix and this
is used to decide what to highlight. Highlighting is also applied when
choosing files.

In the case where the common prefix may be unreasonably large
highlighting is omitted; in this patch the soft limit (above which the
highlighting will be omitted for a particular item) is 0 (in other words,
there is no soft limit) and the hard limit (above which highlighting will
be omitted for all items) is 3, but this can be tweaked.

The actual highlighting is done by the highlight_prefix function, which
will enable us to implement ANSI color code-based highlighting (most
likely using underline or boldface) in the future.

Signed-off-by: Wincent Colaiuta <redacted>
---

I will go now and crawl under a rock.

 git-add--interactive.perl |  110 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index fb1e92a..0e358b5 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -44,7 +44,6 @@ my $status_fmt = '%12s %12s %s';
 my $status_head = sprintf($status_fmt, 'staged', 'unstaged', 'path');
 
 # Returns list of hashes, contents of each of which are:
-# PRINT:	print message
 # VALUE:	pathname
 # BINARY:	is a binary path
 # INDEX:	is index different from HEAD?

@@ -122,8 +121,6 @@ sub list_modified {
 		}
 		push @return, +{
 			VALUE => $_,
-			PRINT => (sprintf $status_fmt,
-				  $it->{INDEX}, $it->{FILE}, $_),
 			%$it,
 		};
 	}

@@ -159,10 +156,95 @@ sub find_unique {
 	return $found;
 }
 
+# inserts string into trie and updates count for each character
+sub update_trie {
+	my ($trie, $string) = @_;
+	foreach (split //, $string) {
+		$trie = $trie->{$_} ||= {COUNT => 0};
+		$trie->{COUNT}++;
+	}
+}
+
+# returns an array of tuples (prefix, remainder)
+sub find_unique_prefixes {
+	my @stuff = @_;
+	my @return = ();
+
+	# any single prefix exceeding the soft limit is omitted
+	# if any prefix exceeds the hard limit all are omitted
+	# 0 indicates no limit
+	my $soft_limit = 0;
+	my $hard_limit = 3;
+
+	# build a trie modelling all possible options
+	my %trie;
+	foreach my $print (@stuff) {
+		if ((ref $print) eq 'ARRAY') {
+			$print = $print->[0];
+		}
+		elsif ((ref $print) eq 'HASH') {
+			$print = $print->{VALUE};
+		}
+		update_trie(\%trie, $print);
+		push @return, $print;
+	}
+
+	# use the trie to find the unique prefixes
+	for (my $i = 0; $i < @return; $i++) {
+		my $ret = $return[$i];
+		my @letters = split //, $ret;
+		my %search = %trie;
+		my ($prefix, $remainder);
+		my $j;
+		for ($j = 0; $j < @letters; $j++) {
+			my $letter = $letters[$j];
+			if ($search{$letter}{COUNT} == 1) {
+				$prefix = substr $ret, 0, $j + 1;
+				$remainder = substr $ret, $j + 1;
+				last;
+			}
+			else {
+				my $prefix = substr $ret, 0, $j;
+				return ()
+				    if ($hard_limit && $j + 1 > $hard_limit);
+			}
+			%search = %{$search{$letter}};
+		}
+		if ($soft_limit && $j + 1 > $soft_limit) {
+			$prefix = undef;
+			$remainder = $ret;
+		}
+		$return[$i] = [$prefix, $remainder];
+	}
+	return @return;
+}
+
+# filters out prefixes which have special meaning to list_and_choose()
+sub is_valid_prefix {
+	my $prefix = shift;
+	return (defined $prefix) &&
+	    !($prefix =~ /[\s,]/) && # separators
+	    !($prefix =~ /^-/) &&    # deselection
+	    !($prefix =~ /^\d+/) &&  # selection
+	    ($prefix ne '*');        # "all" wildcard
+}
+
+# given a prefix/remainder tuple return a string with the prefix highlighted
+# for now use square brackets; later might use ANSI colors (underline, bold)
+sub highlight_prefix {
+	my $prefix = shift;
+	my $remainder = shift;
+	return $remainder unless defined $prefix;
+	return is_valid_prefix($prefix) ?
+	    "[$prefix]$remainder" :
+	    "$prefix$remainder";
+}
+
 sub list_and_choose {
 	my ($opts, @stuff) = @_;
 	my (@chosen, @return);
 	my $i;
+	my @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};
 
       TOPLOOP:
 	while (1) {

@@ -177,13 +259,21 @@ sub list_and_choose {
 		for ($i = 0; $i < @stuff; $i++) {
 			my $chosen = $chosen[$i] ? '*' : ' ';
 			my $print = $stuff[$i];
-			if (ref $print) {
-				if ((ref $print) eq 'ARRAY') {
-					$print = $print->[0];
-				}
-				else {
-					$print = $print->{PRINT};
-				}
+			my $ref = ref $print;
+			my $highlighted = highlight_prefix(@{$prefixes[$i]})
+			    if @prefixes;
+			if ($ref eq 'ARRAY') {
+				$print = $highlighted || $print->[0];
+			}
+			elsif ($ref eq 'HASH') {
+				my $value = $highlighted || $print->{VALUE};
+				$print = sprintf($status_fmt,
+				    $print->{INDEX},
+				    $print->{FILE},
+				    $value);
+			}
+			else {
+				$print = $highlighted || $print;
 			}
 			printf("%s%2d: %s", $chosen, $i+1, $print);
 			if (($opts->{LIST_FLAT}) &&

-- 
1.5.3.6.953.gdffc

El 1/12/2007, a las 3:36, Junio C Hamano escribió:

When list_and_choose() function is letting you choose more than one
items, its prompt becomes ">> ", instead of "> " that is used for a
singleton choice.  To that prompt, you can say "3-7" (Add these 5  
items
to the choice), "*" (I want all of them), "-2-4" (exclude 2 and 3  
and 4
from the set I have chosen so far).  These are also "hidden", and need
to be documented, but that would be a separate patch.

I was just about to prepare some documentation for this when I saw  
that it is already documented, and by none other than you, Junio! (see  
6a5ad23d).

Unless by "documentation" you meant to somehow expose these in the  
interface at runtime... something like this? (applied on top of the  
patch I just sent to the list):

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 32fb9ea..e7b07ee 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -237,7 +237,8 @@ sub is_valid_prefix {
  	    !($prefix =~ /[\s,]/) && # separators
  	    !($prefix =~ /^-/) &&    # deselection
  	    !($prefix =~ /^\d+/) &&  # selection
-	    ($prefix ne '*');        # "all" wildcard
+	    ($prefix ne '*') &&      # "all" wildcard
+	    ($prefix ne '?');        # prompt help
  }

  # given a prefix/remainder tuple return a string with the prefix  

highlighted

@@ -308,7 +309,7 @@ sub list_and_choose {
  			print "> ";
  		}
  		else {
-			print ">> ";
+			print " (?)>> ";
  		}
  		my $line = <STDIN>;
  		if (!$line) {

@@ -318,6 +319,10 @@ sub list_and_choose {
  		}
  		chomp $line;
  		last if $line eq '';
+		if ($line eq '?' && !$opts->{SINGLETON}) {
+			prompt_help_cmd();
+			next TOPLOOP;
+		}
  		for my $choice (split(/[\s,]+/, $line)) {
  			my $choose = 1;
  			my ($bottom, $top);

@@ -363,6 +368,19 @@ sub list_and_choose {
  	return @return;
  }

+sub prompt_help_cmd {
+	print <<\EOF ;
+Prompt help:
+1          - select a single item
+3-5        - select a range of items
+2-3,6-9    - select multiple ranges
+foo        - select item based on unique prefix
+-...       - unselect specified items
+*          - choose all items
+           - (empty) finish selecting
+EOF
+}
+
  sub status_cmd {
  	list_and_choose({ LIST_ONLY => 1, HEADER => $status_head },
  			list_modified());

Cheers,
Wincent