Re: [PATCH v2] add -p: fix 2.17.0-rc* regression due to moved code
- From
- Phillip Wood <phillip.wood@talktalk.net>
- Date
- Mar 31, 2018, 16:26 UTC
- Message-ID
- <f60a2014-3b02-3eae-76cb-950330113afa@talktalk.net>
- In-Reply-To
- <20180331125058.4506-1-avarab@gmail.com>
On 31/03/18 13:50, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> Fix a regression in 88f6ffc1c2 ("add -p: only bind search key if
> there's more than one hunk", 2018-02-13) which is present in
> 2.17.0-rc*, but not 2.16.0.
>
> In Perl, regex variables like $1 always refer to the last regex
> match. When the aforementioned change added a new regex match between
> the old match and the corresponding code that was expecting $1, the $1
> variable would always be undef, since the newly inserted regex match
> doesn't have any captures.
>
> As a result the "/" feature to search for a string in a hunk by regex
> completely broke, on git.git:Good catch, I could have sworn I'd tested my patch but I obviously didn't notice the warning (I've got interactive.singlekey set so it prints the warning and then prompts as it always has done). Calling it completely broken is perhaps a little harsh as it does work if you enter the regex again and with interactive.singlekey set you only have to enter the regex once.
Thanks for fixing it
Phillip
Show 44 quoted lines
>
> $ perl -pi -e 's/Git/Tig/g' README.md
> $ ./git --exec-path=$PWD add -p
> [..]
> Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? s
> Split into 4 hunks.
> [...]
> Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? /Many
> Use of uninitialized value $1 in string eq at /home/avar/g/git/git-add--interactive line 1568, <STDIN> line 1.
> search for regex? Many
>
> I.e. the initial "/regex" command wouldn't work, and would always emit
> a warning and ask again for a regex, now it works as intended again.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> Of course I just noticed the grammar errors in the commit message
> after sending. Here's a v2 with that fixed, also genreated the patch
> with -U6 to make it clear what's going on.
>
> git-add--interactive.perl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index d190469cd8..c1f52e457f 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1561,13 +1561,13 @@ sub patch_update_file {
> elsif ($line =~ m|^/(.*)|) {
> my $regex = $1;
> unless ($other =~ m|/|) {
> error_msg __("No other hunks to search\n");
> next;
> }
> - if ($1 eq "") {
> + if ($regex eq "") {
> print colored $prompt_color, __("search for regex? ");
> $regex = <STDIN>;
> if (defined $regex) {
> chomp $regex;
> }
> }
>