[PATCH/RFC] git-add--interactive.perl: Answer questions with a single keypress

[PATCH/RFC] git-add--interactive.perl: Answer questions with a single keypress

[Suraj N. Kurapati]

From aef6df163b90b9485da1f97a14ffab6d8de9b047 Mon Sep 17 00:00:00 2001
From: Suraj N. Kurapati <sunaku@gmail.com>
Date: Tue, 4 Nov 2008 20:32:34 -0800
Subject: [PATCH] git-add--interactive.perl: Answer questions with a single keypress

Allows the user to answer 'Stage this hunk' questions with a
single keypress, just like in Darcs.  Previously, the user
was forced to press the Return key after every choice, which
quickly becomes tiring and burdensome work for the fingers.

Signed-off-by: Suraj N. Kurapati <sunaku@gmail.com>
---
 git-add--interactive.perl |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index b0223c3..1a22968 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -877,6 +877,7 @@ sub patch_update_file {
 	$num = scalar @hunk;
 	$ix = 0;
 
+	require Term::ReadKey;
 	while (1) {
 		my ($prev, $next, $other, $undecided, $i);
 		$other = '';
@@ -920,7 +921,10 @@ sub patch_update_file {
 			print;
 		}
 		print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
-		my $line = <STDIN>;
+
+		Term::ReadKey::ReadMode('raw');
+		my $line = Term::ReadKey::ReadKey(0);
+		Term::ReadKey::ReadMode('restore');
 		if ($line) {
 			if ($line =~ /^y/i) {
 				$hunk[$ix]{USE} = 1;
-- 
1.6.0.3

\0

Re: [PATCH/RFC v2] git-add--interactive.perl: Answer questions with a single keypress

[Suraj N. Kurapati]

Allows the user to answer 'Stage this hunk' questions with a
single keypress, just like in Darcs.  Previously, the user was
forced to press the Return key after every choice they made.
This quickly becomes tiring, burdensome work for the fingers.

Signed-off-by: Suraj N. Kurapati <sunaku@gmail.com>
---
 git-add--interactive.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index b0223c3..a824984 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -877,6 +877,8 @@ sub patch_update_file {
 	$num = scalar @hunk;
 	$ix = 0;
 
+	require Term::ReadKey;
+	Term::ReadKey::ReadMode('raw');
 	while (1) {
 		my ($prev, $next, $other, $undecided, $i);
 		$other = '';
@@ -920,7 +922,7 @@ sub patch_update_file {
 			print;
 		}
 		print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
-		my $line = <STDIN>;
+		my $line = Term::ReadKey::ReadKey(0);
 		if ($line) {
 			if ($line =~ /^y/i) {
 				$hunk[$ix]{USE} = 1;
@@ -998,6 +1000,7 @@ sub patch_update_file {
 			}
 		}
 	}
+	Term::ReadKey::ReadMode('restore');
 
 	my $n_lofs = 0;
 	my @result = ();
-- 
1.6.0.3

\0

Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress

[Suraj N. Kurapati]

Allows the user to answer 'Stage this hunk' questions with a
single keypress, just like in Darcs.  Previously, the user was
forced to press the Return key after every choice they made.
This quickly becomes tiring, burdensome work for the fingers.

Signed-off-by: Suraj N. Kurapati <sunaku@gmail.com>
---
 git-add--interactive.perl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index b0223c3..b71905e 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -877,6 +877,8 @@ sub patch_update_file {
 	$num = scalar @hunk;
 	$ix = 0;
 
+	require Term::ReadKey;
+	Term::ReadKey::ReadMode('cbreak');
 	while (1) {
 		my ($prev, $next, $other, $undecided, $i);
 		$other = '';
@@ -920,7 +922,7 @@ sub patch_update_file {
 			print;
 		}
 		print colored $prompt_color, "Stage this hunk [y/n/a/d$other/?]? ";
-		my $line = <STDIN>;
+		my $line = Term::ReadKey::ReadKey(0);
 		if ($line) {
 			if ($line =~ /^y/i) {
 				$hunk[$ix]{USE} = 1;
@@ -998,6 +1000,7 @@ sub patch_update_file {
 			}
 		}
 	}
+	Term::ReadKey::ReadMode('restore');
 
 	my $n_lofs = 0;
 	my @result = ();
-- 
1.6.0.3

\0

Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress

[Jeff King]

On Wed, Nov 05, 2008 at 09:59:25AM -0800, Suraj N. Kurapati wrote:

> Allows the user to answer 'Stage this hunk' questions with a
> single keypress, just like in Darcs.  Previously, the user was
> forced to press the Return key after every choice they made.
> This quickly becomes tiring, burdensome work for the fingers.

I think this is a reasonable goal, but I have a few questions/concerns.

 - There are three versions of your patch, but nobody has commented.
   Clearly we can see what changed, but it is not clear what advantage
   one patch has over the other. Care to elaborate?

 - Term::ReadKey, while common, is not part of base perl. So I think
   using it needs to be conditional, and on systems without it we can
   degrade to the current behavior.

 - There's no facility in your patch for restoring the terminal if we
   break out of the loop in an unexpected way (e.g., via the user
   hitting ^C).

 - This only enhances one particular input, the patch loop. It is
   probably worth being consistent and allowing these behavior for other
   menus (though the numeric inputs are a bit trickier).

-Peff

Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress

[Suraj N. Kurapati]

On Thursday 06 November 2008 00:42:36 Jeff King wrote:
> On Wed, Nov 05, 2008 at 09:59:25AM -0800, Suraj N. Kurapati wrote:
> > Allows the user to answer 'Stage this hunk' questions with a
> > single keypress, just like in Darcs.  Previously, the user was
> > forced to press the Return key after every choice they made.
> > This quickly becomes tiring, burdensome work for the fingers.
>
> I think this is a reasonable goal, but I have a few questions/concerns.
>
>  - There are three versions of your patch, but nobody has commented.
>    Clearly we can see what changed, but it is not clear what advantage
>    one patch has over the other. Care to elaborate?

v1 and v2 make the mistake of setting raw mode, which prevent the user from 
pressing Control-C to exit the program.  v3 fixes this by using cbreak mode.

>  - Term::ReadKey, while common, is not part of base perl. So I think
>    using it needs to be conditional, and on systems without it we can
>    degrade to the current behavior.

The git-svn.perl script also uses Term::ReadKey.  Since it is already in the 
git source repository, I thought it was okay to use Term::ReadKey.

>  - There's no facility in your patch for restoring the terminal if we
>    break out of the loop in an unexpected way (e.g., via the user
>    hitting ^C).

Good point.  I'll try to address this in a v4 patch.

>  - This only enhances one particular input, the patch loop. It is
>    probably worth being consistent and allowing these behavior for other
>    menus (though the numeric inputs are a bit trickier).

Understood.

Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress

[Junio C Hamano]

"Suraj N. Kurapati" <sunaku@gmail.com> writes:

> On Thursday 06 November 2008 00:42:36 Jeff King wrote:
>> On Wed, Nov 05, 2008 at 09:59:25AM -0800, Suraj N. Kurapati wrote:
>> > Allows the user to answer 'Stage this hunk' questions with a
>> > single keypress, just like in Darcs.  Previously, the user was
>> > forced to press the Return key after every choice they made.
>> > This quickly becomes tiring, burdensome work for the fingers.
>>
>> I think this is a reasonable goal, but I have a few questions/concerns.
>>
>>  - There are three versions of your patch, but nobody has commented.
>>    Clearly we can see what changed, but it is not clear what advantage
>>    one patch has over the other. Care to elaborate?
>
> v1 and v2 make the mistake of setting raw mode, which prevent the user from 
> pressing Control-C to exit the program.  v3 fixes this by using cbreak mode.

When I was reading the last round I noticed the above change myself, but
it is customary to write this kind of "patch changelog" under --- line in
your patch around here.  That way you can save everybody's time.

Thanks.  And thanks, Jeff, for being a good reviewer.
 

Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress

[Jeff King]

On Thu, Nov 06, 2008 at 08:15:16AM -0800, Suraj N. Kurapati wrote:

> v1 and v2 make the mistake of setting raw mode, which prevent the user from 
> pressing Control-C to exit the program.  v3 fixes this by using cbreak mode.

OK, makes sense.

> The git-svn.perl script also uses Term::ReadKey.  Since it is already in the 
> git source repository, I thought it was okay to use Term::ReadKey.

We are not always consistent with such things, and git-add--interactive
is a little "more core" than git-svn, I think. Still, maybe I am being
overly cautious. We can always try it and see who complains. ;)

I think considering this improvement:

> >  - This only enhances one particular input, the patch loop. It is
> >    probably worth being consistent and allowing these behavior for other
> >    menus (though the numeric inputs are a bit trickier).
> 
> Understood.

it makes sense to factor the ReadKey procedure into a separate function
anyway. At which point it should be easy enough to substitute in the
original behavior if there is no ReadKey.

Side note: I don't know if such people exist, but I suppose it is
possible that somebody would _prefer_ the old behavior. In which case
factoring it out will also make it easier to predicate the behavior on a
config variable, if people want that.

-Peff

Re: [PATCH/RFC v3] git add -i: Answer questions with a single keypress

[Jeff King]

On Thu, Nov 06, 2008 at 09:06:59AM -0800, Junio C Hamano wrote:

> Thanks.  And thanks, Jeff, for being a good reviewer.

You're welcome. I'm trying to lead by example. :)

-Peff