{"thread":{"id":"17517","subject":"[PATCH 1/4] add -p: change prompt separator for 'g'","startedAt":"2009-02-02T21:46:27Z","lastAt":"2009-02-04T20:40:57Z","messageCount":13,"participants":["Thomas Rast","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"102899","messageId":"1233611191-18590-1-git-send-email-trast@student.ethz.ch","threadId":"17517","inReplyTo":"7vljspjk8n.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/4] add -p: Term::ReadKey and more","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-02T21:46:27Z","receivedAt":"2009-02-02T21:46:27Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"I added some more patches. 3/4 is the previous version, with code\nadded to make / work (it just prompts for a regex) and a configuration\nvariable added for Junio:\n\nJunio C Hamano wrote:\n> I'd appreciate if you can allow this \"single stroke\" mode to be optionally\n> turned off by the end user.\n\nI decided to disable it by default since it is an inconsistency\nbetween add -i and add -p for users of both, so if you want this\nfeature, make sure to enable interactive.readkey. (I don't mind the\ninconsistency as I never use 'add -i'.)\n\nI also removed the stray %control_chars from back when my draft\nversions used raw instead of cbreak mode, and consequently had to test\nfor Ctrl-C themselves.\n\n\nFinally, the first two patches fix small problems I saw while looking\nover the code, and 4/4 changes the color of error messages caused by\nkeyboard input to color.interactive.help, lest they drown in the hunk\nthat is automatically redisplayed.\n\n\nThomas Rast (4):\n add -p: change prompt separator for 'g'\n add -p: trap Ctrl-D in 'goto' mode\n add -p: prompt for single characters\n add -p: print errors in help colors\n\n Documentation/config.txt | 6 ++++\n git-add--interactive.perl | 70 ++++++++++++++++++++++++++++++++++++--------\n 2 files changed, 63 insertions(+), 13 deletions(-)\n"},{"id":"102898","messageId":"1233611191-18590-2-git-send-email-trast@student.ethz.ch","threadId":"17517","inReplyTo":"1233611191-18590-1-git-send-email-trast@student.ethz.ch","subject":"[PATCH 1/4] add -p: change prompt separator for 'g'","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-02T21:46:28Z","receivedAt":"2009-02-02T21:46:28Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"57886bc (git-add -i/-p: Change prompt separater from slash to comma,\n2008-11-27) changed the prompt separator to ',', but forgot to adapt\nthe 'g' (goto) command.\n\nSigned-off-by: Thomas Rast \n---\n git-add--interactive.perl | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 30ddab2..551b447 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -948,7 +948,7 @@ sub patch_update_file {\n \t\t\t$other .= ',J';\n \t\t}\n \t\tif ($num > 1) {\n-\t\t\t$other .= '/g';\n+\t\t\t$other .= ',g';\n \t\t}\n \t\tfor ($i = 0; $i < $num; $i++) {\n \t\t\tif (!defined $hunk[$i]{USE}) {\n-- \n1.6.1.2.513.g04677\n"},{"id":"102902","messageId":"1233611191-18590-3-git-send-email-trast@student.ethz.ch","threadId":"17517","inReplyTo":"1233611191-18590-2-git-send-email-trast@student.ethz.ch","subject":"[PATCH 2/4] add -p: trap Ctrl-D in 'goto' mode","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-02T21:46:29Z","receivedAt":"2009-02-02T21:46:29Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"If the user hit Ctrl-D (EOF) while the script was in 'go to hunk?'\nmode, it threw an undefined variable error. Explicitly test for EOF\nand have it re-enter the goto prompt loop.\n\nSigned-off-by: Thomas Rast \n---\n git-add--interactive.perl | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 551b447..3bf0cda 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -994,6 +994,9 @@ sub patch_update_file {\n \t\t\t\t\t}\n \t\t\t\t\tprint \"go to which hunk$extra? \";\n \t\t\t\t\t$response = ;\n+\t\t\t\t\tif (!defined $response) {\n+\t\t\t\t\t\t$response = '';\n+\t\t\t\t\t}\n \t\t\t\t\tchomp $response;\n \t\t\t\t}\n \t\t\t\tif ($response !~ /^\\s*\\d+\\s*$/) {\n-- \n1.6.1.2.513.g04677\n"},{"id":"102900","messageId":"1233611191-18590-4-git-send-email-trast@student.ethz.ch","threadId":"17517","inReplyTo":"1233611191-18590-3-git-send-email-trast@student.ethz.ch","subject":"[PATCH 3/4] add -p: optionally prompt for single characters","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-02T21:46:30Z","receivedAt":"2009-02-02T21:46:30Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Use Term::ReadKey, if available and enabled with interactive.readkey,\nto let the user answer add -p's prompts by pressing a single key.\nWe're not doing the same in the main 'add -i' interface because file\nselection etc. may expect several characters.\n\nTwo commands take an argument: 'g' can easily cope since it'll just\noffer a choice of chunks. '/' now (unconditionally, even without\nreadkey) offers a chance to enter a regex if none was given.\n\nSigned-off-by: Thomas Rast \n---\n Documentation/config.txt | 6 ++++++\n git-add--interactive.perl | 45 +++++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 47 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7fbf64d..cee64db 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1013,6 +1013,12 @@ instaweb.port::\n \tThe port number to bind the gitweb httpd to. See\n \tlinkgit:git-instaweb[1].\n \n+interactive.readkey::\n+\tAttempt to use Term::ReadKey facilities to allow typing\n+\tone-letter input with a single key. Currently only used by\n+\tthe `\\--patch` mode of linkgit:git-add[1]. Silently disabled\n+\tif Term::ReadKey is not available.\n+\n log.date::\n \tSet default date-time mode for the log command. Setting log.date\n \tvalue is similar to using 'git-log'\\'s --date option. The value is one of the\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 3bf0cda..3aa21db 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -33,6 +33,14 @@ my ($diff_new_color) =\n \n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n+my $use_readkey = 0;\n+if ($repo->config_bool(\"interactive.readkey\")) {\n+\teval {\n+\t\tuse Term::ReadKey;\n+\t\t$use_readkey = 1;\n+\t};\n+}\n+\n sub colored {\n \tmy $color = shift;\n \tmy $string = join(\"\", @_);\n@@ -758,11 +766,32 @@ sub diff_applies {\n \treturn close $fh;\n }\n \n+sub _restore_terminal_and_die {\n+\tReadMode 'restore';\n+\tprint \"\\n\";\n+\texit 1;\n+}\n+\n+sub prompt_single_character {\n+\tif ($use_readkey) {\n+\t\tlocal $SIG{TERM} = \\&_restore_terminal_and_die;\n+\t\tlocal $SIG{INT} = \\&_restore_terminal_and_die;\n+\t\tReadMode 'cbreak';\n+\t\tmy $key = ReadKey 0;\n+\t\tReadMode 'restore';\n+\t\tprint \"$key\" if defined $key;\n+\t\tprint \"\\n\";\n+\t\treturn $key;\n+\t} else {\n+\t\treturn ;\n+\t}\n+}\n+\n sub prompt_yesno {\n \tmy ($prompt) = @_;\n \twhile (1) {\n \t\tprint colored $prompt_color, $prompt;\n-\t\tmy $line = ;\n+\t\tmy $line = prompt_single_character;\n \t\treturn 0 if $line =~ /^n/i;\n \t\treturn 1 if $line =~ /^y/i;\n \t}\n@@ -893,7 +922,7 @@ sub patch_update_file {\n \t\t\tprint @{$mode->{DISPLAY}};\n \t\t\tprint colored $prompt_color,\n \t\t\t\t\"Stage mode change [y/n/a/d/?]? \";\n-\t\t\tmy $line = ;\n+\t\t\tmy $line = prompt_single_character;\n \t\t\tif ($line =~ /^y/i) {\n \t\t\t\t$mode->{USE} = 1;\n \t\t\t\tlast;\n@@ -966,7 +995,7 @@ sub patch_update_file {\n \t\t\tprint;\n \t\t}\n \t\tprint colored $prompt_color, \"Stage this hunk [y,n,a,d,/$other,?]? \";\n-\t\tmy $line = ;\n+\t\tmy $line = prompt_single_character;\n \t\tif ($line) {\n \t\t\tif ($line =~ /^y/i) {\n \t\t\t\t$hunk[$ix]{USE} = 1;\n@@ -1018,9 +1047,17 @@ sub patch_update_file {\n \t\t\t\tnext;\n \t\t\t}\n \t\t\telsif ($line =~ m|^/(.*)|) {\n+\t\t\t\tmy $regex = $1;\n+\t\t\t\tif ($1 eq \"\") {\n+\t\t\t\t\tprint colored $prompt_color, \"search for regex? \";\n+\t\t\t\t\t$regex = ;\n+\t\t\t\t\tif (defined $regex) {\n+\t\t\t\t\t\tchomp $regex;\n+\t\t\t\t\t}\n+\t\t\t\t}\n \t\t\t\tmy $search_string;\n \t\t\t\teval {\n-\t\t\t\t\t$search_string = qr{$1}m;\n+\t\t\t\t\t$search_string = qr{$regex}m;\n \t\t\t\t};\n \t\t\t\tif ($@) {\n \t\t\t\t\tmy ($err,$exp) = ($@, $1);\n-- \n1.6.1.2.513.g04677\n"},{"id":"102901","messageId":"1233611191-18590-5-git-send-email-trast@student.ethz.ch","threadId":"17517","inReplyTo":"1233611191-18590-4-git-send-email-trast@student.ethz.ch","subject":"[PATCH 4/4] add -p: print errors in help colors","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-02T21:46:31Z","receivedAt":"2009-02-02T21:46:31Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Print the error messages that go to STDERR in color.interactive.help.\nWhile it's not really help text, the command help also pops up if an\nunknown command was entered (which is an error), and it lets them\nstand out nicely.\n\nSigned-off-by: Thomas Rast \n---\n git-add--interactive.perl | 20 ++++++++++++--------\n 1 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 3aa21db..fe8f364 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -908,6 +908,10 @@ sub display_hunks {\n \treturn $i;\n }\n \n+sub help_msg {\n+\tprint STDERR colored $help_color, @_;\n+}\n+\n sub patch_update_file {\n \tmy ($ix, $num);\n \tmy $path = shift;\n@@ -1029,11 +1033,11 @@ sub patch_update_file {\n \t\t\t\t\tchomp $response;\n \t\t\t\t}\n \t\t\t\tif ($response !~ /^\\s*\\d+\\s*$/) {\n-\t\t\t\t\tprint STDERR \"Invalid number: '$response'\\n\";\n+\t\t\t\t\thelp_msg \"Invalid number: '$response'\\n\";\n \t\t\t\t} elsif (0 < $response && $response <= $num) {\n \t\t\t\t\t$ix = $response - 1;\n \t\t\t\t} else {\n-\t\t\t\t\tprint STDERR \"Sorry, only $num hunks available.\\n\";\n+\t\t\t\t\thelp_msg \"Sorry, only $num hunks available.\\n\";\n \t\t\t\t}\n \t\t\t\tnext;\n \t\t\t}\n@@ -1062,7 +1066,7 @@ sub patch_update_file {\n \t\t\t\tif ($@) {\n \t\t\t\t\tmy ($err,$exp) = ($@, $1);\n \t\t\t\t\t$err =~ s/ at .*git-add--interactive line \\d+, line \\d+.*$//;\n-\t\t\t\t\tprint STDERR \"Malformed search regexp $exp: $err\\n\";\n+\t\t\t\t\thelp_msg \"Malformed search regexp $exp: $err\\n\";\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t\tmy $iy = $ix;\n@@ -1072,7 +1076,7 @@ sub patch_update_file {\n \t\t\t\t\t$iy++;\n \t\t\t\t\t$iy = 0 if ($iy >= $num);\n \t\t\t\t\tif ($ix == $iy) {\n-\t\t\t\t\t\tprint STDERR \"No hunk matches the given pattern\\n\";\n+\t\t\t\t\t\thelp_msg \"No hunk matches the given pattern\\n\";\n \t\t\t\t\t\tlast;\n \t\t\t\t\t}\n \t\t\t\t}\n@@ -1084,7 +1088,7 @@ sub patch_update_file {\n \t\t\t\t\t$ix--;\n \t\t\t\t}\n \t\t\t\telse {\n-\t\t\t\t\tprint STDERR \"No previous hunk\\n\";\n+\t\t\t\t\thelp_msg \"No previous hunk\\n\";\n \t\t\t\t}\n \t\t\t\tnext;\n \t\t\t}\n@@ -1093,7 +1097,7 @@ sub patch_update_file {\n \t\t\t\t\t$ix++;\n \t\t\t\t}\n \t\t\t\telse {\n-\t\t\t\t\tprint STDERR \"No next hunk\\n\";\n+\t\t\t\t\thelp_msg \"No next hunk\\n\";\n \t\t\t\t}\n \t\t\t\tnext;\n \t\t\t}\n@@ -1106,13 +1110,13 @@ sub patch_update_file {\n \t\t\t\t\t}\n \t\t\t\t}\n \t\t\t\telse {\n-\t\t\t\t\tprint STDERR \"No previous hunk\\n\";\n+\t\t\t\t\thelp_msg \"No previous hunk\\n\";\n \t\t\t\t}\n \t\t\t\tnext;\n \t\t\t}\n \t\t\telsif ($line =~ /^j/) {\n \t\t\t\tif ($other !~ /j/) {\n-\t\t\t\t\tprint STDERR \"No next hunk\\n\";\n+\t\t\t\t\thelp_msg \"No next hunk\\n\";\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n-- \n1.6.1.2.513.g04677\n"},{"id":"102924","messageId":"20090203062437.GB21367@sigill.intra.peff.net","threadId":"17517","inReplyTo":"1233611191-18590-4-git-send-email-trast@student.ethz.ch","subject":"Re: [PATCH 3/4] add -p: optionally prompt for single characters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-03T06:24:37Z","receivedAt":"2009-02-03T06:24:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 02, 2009 at 10:46:30PM +0100, Thomas Rast wrote:\n\n> +interactive.readkey::\n> +\tAttempt to use Term::ReadKey facilities to allow typing\n> +\tone-letter input with a single key. Currently only used by\n> +\tthe `\\--patch` mode of linkgit:git-add[1]. Silently disabled\n> +\tif Term::ReadKey is not available.\n\nMinor nit: the name of this variable implies that it will be used across\nall interactive commands (including any future ones). But the\ndescription is intimately linked with perl. Maybe structure it like\n\"here is what this does in general, but here are some specific caveats\".\nSomething like:\n\n interactive.readkey::\n In interactive programs, allow the user to provide one-letter\n input with a single key (i.e., without hitting enter). Currently\n this is used only by the `\\--patch` mode of linkgit:git-add[1].\n Note that this feature is silently disabled for Perl programs\n (like git-add) if Term::ReadKey is not available.\n\nBut since there is only _one_ place it is used now, and since that\nprogram _is_ a Perl program, maybe this is overengineering.\n\nOther than that, your series looks OK to me. I think it would be nice to\nhave a 5/4 that extends the feature to other menus for the sake of\nconsistency. But like you, I really just use \"git add -p\" myself.\n\n-Peff\n"},{"id":"102939","messageId":"1233651248-26717-1-git-send-email-trast@student.ethz.ch","threadId":"17517","inReplyTo":"20090203062437.GB21367@sigill.intra.peff.net","subject":"[Illustration PATCH] add -i: accept single-keypress input","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-03T08:54:08Z","receivedAt":"2009-02-03T08:54:08Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Demonstrate how to add Term::ReadKey functionality to the main add -i\nprompt function list_and_choose().\n\nNot really great because if several input choices share a common first\ncharacter, it prompts for the _rest_ of the line, meaning the choice\nof the first character cannot be undone again.\n---\n\nJeff King wrote:\n> Minor nit: the name of this variable implies that it will be used across\n> all interactive commands (including any future ones).\n\nThat would be the point :-) Not sure what other tools could be\nadapted however. Mergetool maybe, but I don't use that myself.\n\n> But the\n> description is intimately linked with perl. Maybe structure it like\n> \"here is what this does in general, but here are some specific caveats\".\n[...]\n> But since there is only _one_ place it is used now, and since that\n> program _is_ a Perl program, maybe this is overengineering.\n\nI just figured that when another program/language comes along that\nattempts to do the same, it should be changed. I suppose for shell\nscripts it would read something along the lines of \"if the required\n'stty' calls fail, the feature is silently disabled\".\n\n> Other than that, your series looks OK to me. I think it would be nice to\n> have a 5/4 that extends the feature to other menus for the sake of\n> consistency. But like you, I really just use \"git add -p\" myself.\n\nThis patch (on top of 3/4) does roughly that. As I mentioned, I'm not\nhappy with the prompting behaviour for several characters (e.g. if you\ngo to the [p]atch menu and have several files starting with the same\ncharacter). It would be nicer to start a readline-ish prompt that\nalready has the character filled in, but I don't know how to do that,\nand then I don't care _that_ much.\n\n\n git-add--interactive.perl | 68 ++++++++++++++++++++++++++++++--------------\n 1 files changed, 46 insertions(+), 22 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 3aa21db..f41dd09 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -34,9 +34,11 @@ my ($diff_new_color) =\n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n my $use_readkey = 0;\n+my %control_chars;\n if ($repo->config_bool(\"interactive.readkey\")) {\n \teval {\n \t\tuse Term::ReadKey;\n+\t\t%control_chars = GetControlChars;\n \t\t$use_readkey = 1;\n \t};\n }\n@@ -333,12 +335,55 @@ sub highlight_prefix {\n \treturn \"$prompt_color$prefix$normal_color$remainder\";\n }\n \n+sub _restore_terminal_and_die {\n+\tReadMode 'restore';\n+\tprint \"\\n\";\n+\texit 1;\n+}\n+\n+sub prompt_single_character {\n+\tmy ($single_chars) = @_;\n+\t$single_chars = \".\" unless defined $single_chars;\n+\tif ($use_readkey) {\n+\t\tlocal $SIG{TERM} = \\&_restore_terminal_and_die;\n+\t\tlocal $SIG{INT} = \\&_restore_terminal_and_die;\n+\t\tReadMode 'cbreak';\n+\t\tmy $key = ReadKey 0;\n+\t\tReadMode 'restore';\n+\t\t$key = undef if $key eq $control_chars{EOF};\n+\t\tif ((defined $key) && $key !~ /$single_chars/) {\n+\t\t\tprint colored $prompt_color, $key;\n+\t\t\tmy $rest = ;\n+\t\t\t$rest = '' unless defined $rest;\n+\t\t\treturn $key.$rest;\n+\t\t}\n+\t\tprint \"$key\" if defined $key;\n+\t\tprint \"\\n\";\n+\t\treturn $key;\n+\t} else {\n+\t\treturn ;\n+\t}\n+}\n+\n sub list_and_choose {\n \tmy ($opts, @stuff) = @_;\n \tmy (@chosen, @return);\n \tmy $i;\n \tmy @prefixes = find_unique_prefixes(@stuff) unless $opts->{LIST_ONLY};\n \n+\tmy $single_chars = '';\n+\tfor (@prefixes) {\n+\t\tmy ($pre, $post) = @{$_};\n+\t\tif (length $pre == 1) {\n+\t\t\t$single_chars .= $pre;\n+\t\t}\n+\t}\n+\tfor ($i = 0; $i < 10; $i++) {\n+\t\tif (10*$i > scalar @stuff) {\n+\t\t\t$single_chars .= $i;\n+\t\t}\n+\t}\n+\n TOPLOOP:\n \twhile (1) {\n \t\tmy $last_lf = 0;\n@@ -392,7 +437,7 @@ sub list_and_choose {\n \t\telse {\n \t\t\tprint \">> \";\n \t\t}\n-\t\tmy $line = ;\n+\t\tmy $line = prompt_single_character('['.$single_chars.']');\n \t\tif (!$line) {\n \t\t\tprint \"\\n\";\n \t\t\t$opts->{ON_EOF}->() if $opts->{ON_EOF};\n@@ -766,27 +811,6 @@ sub diff_applies {\n \treturn close $fh;\n }\n \n-sub _restore_terminal_and_die {\n-\tReadMode 'restore';\n-\tprint \"\\n\";\n-\texit 1;\n-}\n-\n-sub prompt_single_character {\n-\tif ($use_readkey) {\n-\t\tlocal $SIG{TERM} = \\&_restore_terminal_and_die;\n-\t\tlocal $SIG{INT} = \\&_restore_terminal_and_die;\n-\t\tReadMode 'cbreak';\n-\t\tmy $key = ReadKey 0;\n-\t\tReadMode 'restore';\n-\t\tprint \"$key\" if defined $key;\n-\t\tprint \"\\n\";\n-\t\treturn $key;\n-\t} else {\n-\t\treturn ;\n-\t}\n-}\n-\n sub prompt_yesno {\n \tmy ($prompt) = @_;\n \twhile (1) {\n-- \n1.6.1.2.513.g04677\n"},{"id":"102941","messageId":"7vbptjdh5e.fsf@gitster.siamese.dyndns.org","threadId":"17517","inReplyTo":"1233651248-26717-1-git-send-email-trast@student.ethz.ch","subject":"Re: [Illustration PATCH] add -i: accept single-keypress input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-03T09:05:49Z","receivedAt":"2009-02-03T09:05:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Demonstrate how to add Term::ReadKey functionality to the main add -i\n> prompt function list_and_choose().\n>\n> Not really great because if several input choices share a common first\n> character, it prompts for the _rest_ of the line, meaning the choice\n> of the first character cannot be undone again.\n\nHmm, you could trigger the action immediately after seeing _enough_ number\nof characters to disambiguate instead of stop-and-prompt, I guess? That\nway, you would get a single-key merely as a degenerate case when the\nchoices are all distinct.\n"},{"id":"102944","messageId":"200902031035.04978.trast@student.ethz.ch","threadId":"17517","inReplyTo":"7vbptjdh5e.fsf@gitster.siamese.dyndns.org","subject":"Re: [Illustration PATCH] add -i: accept single-keypress input","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-03T09:35:00Z","receivedAt":"2009-02-03T09:35:00Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Hmm, you could trigger the action immediately after seeing _enough_ number\n> of characters to disambiguate instead of stop-and-prompt, I guess? That\n> way, you would get a single-key merely as a degenerate case when the\n> choices are all distinct.\n\nI don't think that's very nice. On the one hand, you'd really want to\nallow the user to delete some of the input again if he decides to do\nsomething else instead, and we'd either need readline or need to\nreinvent it for that. On the other hand, some possible choices might\nbe a valid prefix of some _other_ choices, at which point you need a\nterminator (such as the ) anyway. I expect this to be fairly\ncommon since many of the list_and_choose() prompts are numbered, so\nthat 1 and 10 run into this problem.\n\nThen again I don't use add -i, so someone else should say what to do.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"103067","messageId":"7vljsm9492.fsf@gitster.siamese.dyndns.org","threadId":"17517","inReplyTo":"200902031035.04978.trast@student.ethz.ch","subject":"Re: [Illustration PATCH] add -i: accept single-keypress input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T05:10:17Z","receivedAt":"2009-02-04T05:10:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Junio C Hamano wrote:\n>\n>> Hmm, you could trigger the action immediately after seeing _enough_ number\n>> of characters to disambiguate instead of stop-and-prompt, I guess? That\n>> way, you would get a single-key merely as a degenerate case when the\n>> choices are all distinct.\n>\n> I don't think that's very nice. On the one hand, you'd really want to\n> allow the user to delete some of the input again if he decides to do\n> something else instead, and we'd either need readline or need to\n> reinvent it for that....\n\nBut doesn't the original \"single-keypress\" theme shares that problem\nanyway? It trades the ability to \"delete some of the it again if he\ndecides to\" away, in exchange for something else (probably \"quicker input\"\nor \"perceived ease of use\").\n\n> .... On the other hand, some possible choices might\n> be a valid prefix of some _other_ choices, at which point you need a\n> terminator (such as the ) anyway. I expect this to be fairly\n> common since many of the list_and_choose() prompts are numbered, so\n> that 1 and 10 run into this problem.\n\nI think that is independent, is an easy to fix (e.g. 1..9 A B C...Z), and\nis a chicken-and-egg issue. If the input mechanism had \"enough leading\nletters to disambiguate\" semantics from the beginning, I am reasonably\nsure that list_and_choose() would have implemented its choice as not small\nintegers but distinct letters.\n"},{"id":"103088","messageId":"200902040951.50018.trast@student.ethz.ch","threadId":"17517","inReplyTo":"7vljsm9492.fsf@gitster.siamese.dyndns.org","subject":"Re: [Illustration PATCH] add -i: accept single-keypress input","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-04T08:51:44Z","receivedAt":"2009-02-04T08:51:44Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Thomas Rast writes:\n> \n> > Junio C Hamano wrote:\n> >\n> >> Hmm, you could trigger the action immediately after seeing _enough_ number\n> >> of characters to disambiguate instead of stop-and-prompt, I guess? That\n> >> way, you would get a single-key merely as a degenerate case when the\n> >> choices are all distinct.\n> >\n> > I don't think that's very nice. On the one hand, you'd really want to\n> > allow the user to delete some of the input again if he decides to do\n> > something else instead, and we'd either need readline or need to\n> > reinvent it for that....\n> \n> But doesn't the original \"single-keypress\" theme shares that problem\n> anyway? It trades the ability to \"delete some of the it again if he\n> decides to\" away, in exchange for something else (probably \"quicker input\"\n> or \"perceived ease of use\").\n\nI don't think so: With the single-key input, you _know_ that whatever\nkey you press will be executed immediately. In a prefix scheme, the\nprompt might (you don't even know; you'll have to look and check) wait\nfor more input, but you cannot undo the choice even though it has not\nbeen executed yet.\n\n*shrug*\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"103187","messageId":"200902042042.13787.trast@student.ethz.ch","threadId":"17517","inReplyTo":"20090203062437.GB21367@sigill.intra.peff.net","subject":"Re: [PATCH 3/4] add -p: optionally prompt for single characters","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-04T19:42:08Z","receivedAt":"2009-02-04T19:42:08Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> On Mon, Feb 02, 2009 at 10:46:30PM +0100, Thomas Rast wrote:\n> Minor nit: the name of this variable implies that it will be used across\n> all interactive commands (including any future ones). But the\n> description is intimately linked with perl. Maybe structure it like\n> \"here is what this does in general, but here are some specific caveats\".\n> Something like:\n> \n> interactive.readkey::\n> In interactive programs, allow the user to provide one-letter\n> input with a single key (i.e., without hitting enter). Currently\n> this is used only by the `\\--patch` mode of linkgit:git-add[1].\n> Note that this feature is silently disabled for Perl programs\n> (like git-add) if Term::ReadKey is not available.\n\nJunio indicates in the corresponding pu topic that he is of the same\nopinion, so I'll reroll with your help text. (It's somewhat\ninaccurate since git-add is not really a perl program, but let's not\ntell the users about our implementation details.)\n\nMeanwhile I've had regrets about reusing color.interactive.help for\nerrors, so I'll also reroll 4/4 with a new color .error that just\ndefaults to .help.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"103199","messageId":"7v63jqorza.fsf@gitster.siamese.dyndns.org","threadId":"17517","inReplyTo":"200902042042.13787.trast@student.ethz.ch","subject":"Re: [PATCH 3/4] add -p: optionally prompt for single characters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T20:40:57Z","receivedAt":"2009-02-04T20:40:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> Jeff King wrote:\n>> On Mon, Feb 02, 2009 at 10:46:30PM +0100, Thomas Rast wrote:\n>> Minor nit: the name of this variable implies that it will be used across\n>> all interactive commands (including any future ones). But the\n>> description is intimately linked with perl. Maybe structure it like\n>> \"here is what this does in general, but here are some specific caveats\".\n>> Something like:\n>> \n>> interactive.readkey::\n>> In interactive programs, allow the user to provide one-letter\n>> input with a single key (i.e., without hitting enter). Currently\n>> this is used only by the `\\--patch` mode of linkgit:git-add[1].\n>> Note that this feature is silently disabled for Perl programs\n>> (like git-add) if Term::ReadKey is not available.\n>\n> Junio indicates in the corresponding pu topic that he is of the same\n> opinion, so I'll reroll with your help text. (It's somewhat\n> inaccurate since git-add is not really a perl program, but let's not\n> tell the users about our implementation details.)\n\nWe could be even more vague and say \"is silently disabled if the\nunderlying system software does not let it read just a single keystroke in\na portable way\", or something like that.\n\nAnd readkey would be a bad name. You are doing singlekey, and use of\nreadkey *is* an implementation detail, no?\n"}]}