{"thread":{"id":"47824","subject":"[PATCH 3/3] add -p: improve error messages","startedAt":"2018-02-13T10:33:06Z","lastAt":"2018-04-01T05:00:36Z","messageCount":9,"participants":["Phillip Wood","Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"339179","messageId":"20180213103241.6789-4-phillip.wood@talktalk.net","threadId":"47824","inReplyTo":"20180213103241.6789-1-phillip.wood@talktalk.net","subject":"[PATCH 3/3] add -p: improve error messages","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:32:41Z","receivedAt":"2018-02-13T10:33:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf the user presses a key that isn't currently active then explain why\nit isn't active rather than just listing all the keys. It already did\nthis for some keys, this patch does the same for the those that\nweren't already handled.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-add--interactive.perl | 18 +++++++++++++++---\n 1 file changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 79ab36aacf84ed329a9d50886b98bb4ef8b8e312..d9d8ff3090e914421ab67071117789f6b3554475 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1437,8 +1437,12 @@ sub patch_update_file {\n \t\t\t\t}\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\telsif ($other =~ /g/ && $line =~ /^g(.*)/) {\n+\t\t\telsif ($line =~ /^g(.*)/) {\n \t\t\t\tmy $response = $1;\n+\t\t\t\tunless ($other =~ /g/) {\n+\t\t\t\t\terror_msg __(\"No other hunks to goto\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n \t\t\t\tmy $no = $ix > 10 ? $ix - 10 : 0;\n \t\t\t\twhile ($response eq '') {\n \t\t\t\t\t$no = display_hunks(\\@hunk, $no);\n@@ -1556,7 +1560,11 @@ sub patch_update_file {\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n-\t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n+\t\t\telsif ($line =~ /^s/) {\n+\t\t\t\tunless ($other =~ /s/) {\n+\t\t\t\t\terror_msg __(\"Sorry, cannot split this hunk\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n \t\t\t\tif (1 < @split) {\n \t\t\t\t\tprint colored $header_color, sprintf(\n@@ -1568,7 +1576,11 @@ sub patch_update_file {\n \t\t\t\t$num = scalar @hunk;\n \t\t\t\tnext;\n \t\t\t}\n-\t\t\telsif ($other =~ /e/ && $line =~ /^e/) {\n+\t\t\telsif ($line =~ /^e/) {\n+\t\t\t\tunless ($other =~ /e/) {\n+\t\t\t\t\terror_msg __(\"Sorry, cannot edit this hunk\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n \t\t\t\tmy $newhunk = edit_hunk_loop($head, \\@hunk, $ix);\n \t\t\t\tif (defined $newhunk) {\n \t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n-- \n2.16.1\n\n"},{"id":"339180","messageId":"20180213103241.6789-2-phillip.wood@talktalk.net","threadId":"47824","inReplyTo":"20180213103241.6789-1-phillip.wood@talktalk.net","subject":"[PATCH 1/3] add -p: only display help for active keys","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:32:39Z","receivedAt":"2018-02-13T10:33:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf the user presses a key that add -p wasn't expecting then it prints\na list of key bindings. Although the prompt only lists the active\nbindings the help was printed for all bindings.  Fix this by using the\nlist of keys in the prompt to filter the help. Note that the list of\nkeys was already passed to help_patch_cmd() by the caller so there is\nno change needed to the call site.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-add--interactive.perl | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 964c3a75420db4751cf11125b68b6904112632f1..76b941f8f0071b7bfa9d515af25b69997ef4581a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1184,7 +1184,13 @@ d - do not apply this hunk or any of the later hunks in the file\"),\n );\n \n sub help_patch_cmd {\n-\tprint colored $help_color, __($help_patch_modes{$patch_mode}), \"\\n\", __ <<EOF ;\n+\tlocal $_;\n+\tmy $other = $_[0] . \",/,?\";\n+\tprint colored $help_color, __($help_patch_modes{$patch_mode}), \"\\n\",\n+\t\tmap { \"$_\\n\" } grep {\n+\t\t\tmy $c = quotemeta(substr($_, 0, 1));\n+\t\t\t$other =~ /,$c/\n+\t\t} split \"\\n\", __ <<EOF ;\n g - select a hunk to go to\n / - search for a hunk matching the given regex\n j - leave this hunk undecided, see next undecided hunk\n-- \n2.16.1\n\n"},{"id":"339181","messageId":"20180213103241.6789-1-phillip.wood@talktalk.net","threadId":"47824","inReplyTo":null,"subject":"[PATCH 0/3] add -p: improve help messages","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:32:38Z","receivedAt":"2018-02-13T10:33:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nImprove the help displayed if the user presses an inactive key by only\nlisting active keys and printing specific messages for some keys. Also\ndisable search if there's only a single hunk.\n\nPhillip Wood (3):\n  add -p: only display help for active keys\n  add -p: Only bind search key if there's more than one hunk\n  add -p: improve error messages\n\n git-add--interactive.perl | 74 ++++++++++++++++++++++++++++++-----------------\n 1 file changed, 48 insertions(+), 26 deletions(-)\n\n-- \n2.16.1\n\n"},{"id":"339182","messageId":"20180213103241.6789-3-phillip.wood@talktalk.net","threadId":"47824","inReplyTo":"20180213103241.6789-1-phillip.wood@talktalk.net","subject":"[PATCH 2/3] add -p: Only bind search key if there's more than one hunk","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-13T10:32:40Z","receivedAt":"2018-02-13T10:33:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf there is only a single hunk then disable searching as there is\nnothing to search for. Also print a specific error message if the user\ntries to search with '/' when there's only a single hunk rather than\njust listing the key bindings.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-add--interactive.perl | 50 +++++++++++++++++++++++++----------------------\n 1 file changed, 27 insertions(+), 23 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 76b941f8f0071b7bfa9d515af25b69997ef4581a..79ab36aacf84ed329a9d50886b98bb4ef8b8e312 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1185,7 +1185,7 @@ d - do not apply this hunk or any of the later hunks in the file\"),\n \n sub help_patch_cmd {\n \tlocal $_;\n-\tmy $other = $_[0] . \",/,?\";\n+\tmy $other = $_[0] . \",?\";\n \tprint colored $help_color, __($help_patch_modes{$patch_mode}), \"\\n\",\n \t\tmap { \"$_\\n\" } grep {\n \t\t\tmy $c = quotemeta(substr($_, 0, 1));\n@@ -1308,39 +1308,39 @@ sub display_hunks {\n \n my %patch_update_prompt_modes = (\n \tstage => {\n-\t\tmode => N__(\"Stage mode change [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Stage deletion [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Stage this hunk [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Stage mode change [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Stage deletion [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Stage this hunk [y,n,q,a,d%s,?]? \"),\n \t},\n \tstash => {\n-\t\tmode => N__(\"Stash mode change [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Stash deletion [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Stash this hunk [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Stash mode change [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Stash deletion [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Stash this hunk [y,n,q,a,d%s,?]? \"),\n \t},\n \treset_head => {\n-\t\tmode => N__(\"Unstage mode change [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Unstage deletion [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Unstage this hunk [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Unstage mode change [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Unstage deletion [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Unstage this hunk [y,n,q,a,d%s,?]? \"),\n \t},\n \treset_nothead => {\n-\t\tmode => N__(\"Apply mode change to index [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Apply deletion to index [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Apply this hunk to index [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Apply mode change to index [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Apply deletion to index [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Apply this hunk to index [y,n,q,a,d%s,?]? \"),\n \t},\n \tcheckout_index => {\n-\t\tmode => N__(\"Discard mode change from worktree [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Discard deletion from worktree [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Discard this hunk from worktree [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Discard mode change from worktree [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Discard deletion from worktree [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Discard this hunk from worktree [y,n,q,a,d%s,?]? \"),\n \t},\n \tcheckout_head => {\n-\t\tmode => N__(\"Discard mode change from index and worktree [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Discard deletion from index and worktree [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Discard this hunk from index and worktree [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Discard mode change from index and worktree [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Discard deletion from index and worktree [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Discard this hunk from index and worktree [y,n,q,a,d%s,?]? \"),\n \t},\n \tcheckout_nothead => {\n-\t\tmode => N__(\"Apply mode change to index and worktree [y,n,q,a,d,/%s,?]? \"),\n-\t\tdeletion => N__(\"Apply deletion to index and worktree [y,n,q,a,d,/%s,?]? \"),\n-\t\thunk => N__(\"Apply this hunk to index and worktree [y,n,q,a,d,/%s,?]? \"),\n+\t\tmode => N__(\"Apply mode change to index and worktree [y,n,q,a,d%s,?]? \"),\n+\t\tdeletion => N__(\"Apply deletion to index and worktree [y,n,q,a,d%s,?]? \"),\n+\t\thunk => N__(\"Apply this hunk to index and worktree [y,n,q,a,d%s,?]? \"),\n \t},\n );\n \n@@ -1396,7 +1396,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@@ -1484,6 +1484,10 @@ sub patch_update_file {\n \t\t\t}\n \t\t\telsif ($line =~ m|^/(.*)|) {\n \t\t\t\tmy $regex = $1;\n+\t\t\t\tunless ($other =~ m|/|) {\n+\t\t\t\t\terror_msg __(\"No other hunks to search\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\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 = <STDIN>;\n-- \n2.16.1\n\n"},{"id":"343526","messageId":"20180331123605.30076-1-avarab@gmail.com","threadId":"47824","inReplyTo":"20180213103241.6789-3-phillip.wood@talktalk.net","subject":"[PATCH] add -p: fix 2.17.0-rc* regression due to moved code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-31T12:36:05Z","receivedAt":"2018-03-31T12:40:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in 88f6ffc1c2 (\"add -p: only bind search key if\nthere's more than one hunk\", 2018-02-13) which is present in\n2.17.0-rc*, but not 2.16.0.\n\nIn Perl regex variables like $1 always refer to the last regex\nmatch. When the added a new regex match between the old match and the\ncode that was expecting its $1, the $1 variable would always become\nundef.\n\nAs a result the \"/\" feature to search for a string in a hunk by regex\ncompletely broke, on git.git:\n\n    $ perl -pi -e 's/Git/Tig/g' README.md\n    $ ./git --exec-path=$PWD add -p\n    [..]\n    Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? s\n    Split into 4 hunks.\n    [...]\n    Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? /Many\n    Use of uninitialized value $1 in string eq at /home/avar/g/git/git-add--interactive line 1568, <STDIN> line 1.\n    search for regex? Many\n\nI.e. the initial \"/regex\" command wouldn't work, and would always emit\na warning and ask again for a regex, now it works as intended again.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nJunio: You'll want this for 2.17.0 final. I didn't have time to add\ntests for this, but looking over t3701-add-interactive.sh it seems\ndoable. Phillip: Maybe something you'd be interested in?\n\n git-add--interactive.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex d190469cd8..c1f52e457f 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1564,7 +1564,7 @@ sub patch_update_file {\n \t\t\t\t\terror_msg __(\"No other hunks to search\\n\");\n \t\t\t\t\tnext;\n \t\t\t\t}\n-\t\t\t\tif ($1 eq \"\") {\n+\t\t\t\tif ($regex eq \"\") {\n \t\t\t\t\tprint colored $prompt_color, __(\"search for regex? \");\n \t\t\t\t\t$regex = <STDIN>;\n \t\t\t\t\tif (defined $regex) {\n-- \n2.17.0.rc1.321.gba9d0f2565\n\n"},{"id":"343527","messageId":"20180331125058.4506-1-avarab@gmail.com","threadId":"47824","inReplyTo":"20180331123605.30076-1-avarab@gmail.com","subject":"[PATCH v2] add -p: fix 2.17.0-rc* regression due to moved code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-31T12:50:58Z","receivedAt":"2018-03-31T12:51:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in 88f6ffc1c2 (\"add -p: only bind search key if\nthere's more than one hunk\", 2018-02-13) which is present in\n2.17.0-rc*, but not 2.16.0.\n\nIn Perl, regex variables like $1 always refer to the last regex\nmatch. When the aforementioned change added a new regex match between\nthe old match and the corresponding code that was expecting $1, the $1\nvariable would always be undef, since the newly inserted regex match\ndoesn't have any captures.\n\nAs a result the \"/\" feature to search for a string in a hunk by regex\ncompletely broke, on git.git:\n\n    $ perl -pi -e 's/Git/Tig/g' README.md\n    $ ./git --exec-path=$PWD add -p\n    [..]\n    Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? s\n    Split into 4 hunks.\n    [...]\n    Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? /Many\n    Use of uninitialized value $1 in string eq at /home/avar/g/git/git-add--interactive line 1568, <STDIN> line 1.\n    search for regex? Many\n\nI.e. the initial \"/regex\" command wouldn't work, and would always emit\na warning and ask again for a regex, now it works as intended again.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nOf course I just noticed the grammar errors in the commit message\nafter sending. Here's a v2 with that fixed, also genreated the patch\nwith -U6 to make it clear what's going on.\n\n git-add--interactive.perl | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex d190469cd8..c1f52e457f 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1561,13 +1561,13 @@ sub patch_update_file {\n \t\t\telsif ($line =~ m|^/(.*)|) {\n \t\t\t\tmy $regex = $1;\n \t\t\t\tunless ($other =~ m|/|) {\n \t\t\t\t\terror_msg __(\"No other hunks to search\\n\");\n \t\t\t\t\tnext;\n \t\t\t\t}\n-\t\t\t\tif ($1 eq \"\") {\n+\t\t\t\tif ($regex eq \"\") {\n \t\t\t\t\tprint colored $prompt_color, __(\"search for regex? \");\n \t\t\t\t\t$regex = <STDIN>;\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-- \n2.17.0.rc1.321.gba9d0f2565\n\n"},{"id":"343536","messageId":"f60a2014-3b02-3eae-76cb-950330113afa@talktalk.net","threadId":"47824","inReplyTo":"20180331125058.4506-1-avarab@gmail.com","subject":"Re: [PATCH v2] add -p: fix 2.17.0-rc* regression due to moved code","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-31T16:26:22Z","receivedAt":"2018-03-31T16:26:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 31/03/18 13:50, Ævar Arnfjörð Bjarmason wrote:\n> Fix a regression in 88f6ffc1c2 (\"add -p: only bind search key if\n> there's more than one hunk\", 2018-02-13) which is present in\n> 2.17.0-rc*, but not 2.16.0.\n> \n> In Perl, regex variables like $1 always refer to the last regex\n> match. When the aforementioned change added a new regex match between\n> the old match and the corresponding code that was expecting $1, the $1\n> variable would always be undef, since the newly inserted regex match\n> doesn't have any captures.\n> \n> As a result the \"/\" feature to search for a string in a hunk by regex\n> completely broke, on git.git:\n\nGood catch, I could have sworn I'd tested my patch but I obviously \ndidn't notice the warning (I've got interactive.singlekey set so it \nprints the warning and then prompts as it always has done). Calling it \ncompletely broken is perhaps a little harsh as it does work if you enter \nthe regex again and with interactive.singlekey set you only have to \nenter the regex once.\n\nThanks for fixing it\n\nPhillip\n> \n>      $ perl -pi -e 's/Git/Tig/g' README.md\n>      $ ./git --exec-path=$PWD add -p\n>      [..]\n>      Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? s\n>      Split into 4 hunks.\n>      [...]\n>      Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? /Many\n>      Use of uninitialized value $1 in string eq at /home/avar/g/git/git-add--interactive line 1568, <STDIN> line 1.\n>      search for regex? Many\n> \n> I.e. the initial \"/regex\" command wouldn't work, and would always emit\n> a warning and ask again for a regex, now it works as intended again.\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n> \n> Of course I just noticed the grammar errors in the commit message\n> after sending. Here's a v2 with that fixed, also genreated the patch\n> with -U6 to make it clear what's going on.\n> \n>   git-add--interactive.perl | 2 +-\n>   1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index d190469cd8..c1f52e457f 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1561,13 +1561,13 @@ sub patch_update_file {\n>   \t\t\telsif ($line =~ m|^/(.*)|) {\n>   \t\t\t\tmy $regex = $1;\n>   \t\t\t\tunless ($other =~ m|/|) {\n>   \t\t\t\t\terror_msg __(\"No other hunks to search\\n\");\n>   \t\t\t\t\tnext;\n>   \t\t\t\t}\n> -\t\t\t\tif ($1 eq \"\") {\n> +\t\t\t\tif ($regex eq \"\") {\n>   \t\t\t\t\tprint colored $prompt_color, __(\"search for regex? \");\n>   \t\t\t\t\t$regex = <STDIN>;\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> \n\n"},{"id":"343539","messageId":"87bmf4w4l7.fsf@evledraar.gmail.com","threadId":"47824","inReplyTo":"f60a2014-3b02-3eae-76cb-950330113afa@talktalk.net","subject":"Re: [PATCH v2] add -p: fix 2.17.0-rc* regression due to moved code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-31T17:13:40Z","receivedAt":"2018-03-31T17:20:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Mar 31 2018, Phillip Wood wrote:\n\n> On 31/03/18 13:50, Ævar Arnfjörð Bjarmason wrote:\n>> Fix a regression in 88f6ffc1c2 (\"add -p: only bind search key if\n>> there's more than one hunk\", 2018-02-13) which is present in\n>> 2.17.0-rc*, but not 2.16.0.\n>>\n>> In Perl, regex variables like $1 always refer to the last regex\n>> match. When the aforementioned change added a new regex match between\n>> the old match and the corresponding code that was expecting $1, the $1\n>> variable would always be undef, since the newly inserted regex match\n>> doesn't have any captures.\n>>\n>> As a result the \"/\" feature to search for a string in a hunk by regex\n>> completely broke, on git.git:\n>\n> Good catch, I could have sworn I'd tested my patch but I obviously\n> didn't notice the warning (I've got interactive.singlekey set so it\n> prints the warning and then prompts as it always has done). Calling it\n> completely broken is perhaps a little harsh as it does work if you\n> enter the regex again and with interactive.singlekey set you only have\n> to enter the regex once.\n\nTo clarify by \"completely broken\" I mean the \"/\" feature itself, but\nyeah, you can still search by regex since we just so happen to have the\nfallback codepath intended to catch \"/\" without an accompanying string,\nwhich'll kick in and ask you for the regex since $1 will be undef at\nthat point, and will thus coerce stringwise to \"\".\n\n> Thanks for fixing it\n>\n> Phillip\n>>\n>>      $ perl -pi -e 's/Git/Tig/g' README.md\n>>      $ ./git --exec-path=$PWD add -p\n>>      [..]\n>>      Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? s\n>>      Split into 4 hunks.\n>>      [...]\n>>      Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? /Many\n>>      Use of uninitialized value $1 in string eq at /home/avar/g/git/git-add--interactive line 1568, <STDIN> line 1.\n>>      search for regex? Many\n>>\n>> I.e. the initial \"/regex\" command wouldn't work, and would always emit\n>> a warning and ask again for a regex, now it works as intended again.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n>> ---\n>>\n>> Of course I just noticed the grammar errors in the commit message\n>> after sending. Here's a v2 with that fixed, also genreated the patch\n>> with -U6 to make it clear what's going on.\n>>\n>>   git-add--interactive.perl | 2 +-\n>>   1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n>> index d190469cd8..c1f52e457f 100755\n>> --- a/git-add--interactive.perl\n>> +++ b/git-add--interactive.perl\n>> @@ -1561,13 +1561,13 @@ sub patch_update_file {\n>>   \t\t\telsif ($line =~ m|^/(.*)|) {\n>>   \t\t\t\tmy $regex = $1;\n>>   \t\t\t\tunless ($other =~ m|/|) {\n>>   \t\t\t\t\terror_msg __(\"No other hunks to search\\n\");\n>>   \t\t\t\t\tnext;\n>>   \t\t\t\t}\n>> -\t\t\t\tif ($1 eq \"\") {\n>> +\t\t\t\tif ($regex eq \"\") {\n>>   \t\t\t\t\tprint colored $prompt_color, __(\"search for regex? \");\n>>   \t\t\t\t\t$regex = <STDIN>;\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>>\n"},{"id":"343549","messageId":"xmqqbmf31pxx.fsf@gitster-ct.c.googlers.com","threadId":"47824","inReplyTo":"20180331125058.4506-1-avarab@gmail.com","subject":"Re: [PATCH v2] add -p: fix 2.17.0-rc* regression due to moved code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-01T05:00:26Z","receivedAt":"2018-04-01T05:00:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index d190469cd8..c1f52e457f 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1561,13 +1561,13 @@ sub patch_update_file {\n>  \t\t\telsif ($line =~ m|^/(.*)|) {\n>  \t\t\t\tmy $regex = $1;\n>  \t\t\t\tunless ($other =~ m|/|) {\n>  \t\t\t\t\terror_msg __(\"No other hunks to search\\n\");\n>  \t\t\t\t\tnext;\n>  \t\t\t\t}\n> -\t\t\t\tif ($1 eq \"\") {\n> +\t\t\t\tif ($regex eq \"\") {\n>  \t\t\t\t\tprint colored $prompt_color, __(\"search for regex? \");\n\nAh.  That \"unless ... { }\" thing is what was inserted recently, and\nthe patch is an obvious fix once the problem is pointed out.  Thanks\nfor a careful reading.\n\nIt makes me wonder what the original author, who already captured $1\nin $regex, was thinking when he wrote the comparison with $1 there,\nbut that is OK---ithappend long time ago in early 2009.\n\nWill apply.  Thanks.\n\n>  \t\t\t\t\t$regex = <STDIN>;\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"}]}