Re: [PATCH v2] git-jump: make `diff` work with filenames containing spaces

From: Jeff King <peff@peff.net>
Date: Aug 14, 2025, 23:18 UTC
Message-ID: <20250814231849.GB2937@coredump.intra.peff.net>
In-Reply-To: <pull.1950.v2.git.1754913323810.gitgitgadget@gmail.com>

On Mon, Aug 11, 2025 at 11:55:23AM +0000, Greg Hurrell via GitGitGadget wrote:

Show 15 quoted lines

> This commit takes the simple approach of unconditionally stripping any
> trailing tab. Consider the following three examples:
> 
> 1. For file "foo", Git will emit "foo".
> 2. For file "foo bar", Git will emit "foo bar\t".
> 3. For file "foo\t", Git will emit "\"foo\t\"".
> 4. For file "foo bar\t", Git will emit "\"foo bar\t\"".
> 
> Before this commit, `git-jump` correctly handled only case "1".
> 
> After this commit, `git-jump` correctly handles cases "1" and "2". In
> reality, these are the only cases people are going to run into with any
> regularity, and the other two are rare edge cases, which probably aren't
> worth the effort to support unless somebody actually complains about
> them.

Thanks for laying out these cases. I think this list shows that we are making things strictly better, but just stopping short of handling unquoting. So it seems like a no-brainer to take this patch (though I think even with the description in v1, in which we thought we might regress "foo\t", I think it would still have been worth it).

And I think this is a good stopping point. Handling unquoting would be tricky for a case that is unlikely to come up in practice. And I'm not even sure we could make it foolproof, anyway. We're feeding this to the editor's quickfix parser, so I'm not sure how we'd represent something like a newline. We'd have to agree on the quoting scheme with the editor, and from my (admittedly brief) research, there is not even a mechanism in vim for that.

Show 13 quoted lines

> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump
> index 3f696759617..8d1d5d79a69 100755
> --- a/contrib/git-jump/git-jump
> +++ b/contrib/git-jump/git-jump
> @@ -44,7 +44,7 @@ open_editor() {
>  mode_diff() {
>  	git diff --no-prefix --relative "$@" |
>  	perl -ne '
> -	if (m{^\+\+\+ (.*)}) { $file = $1 eq "/dev/null" ? undef : $1; next }
> +	if (m{^\+\+\+ (.*?)\t?$}) { $file = $1 eq "/dev/null" ? undef : $1; next }
>  	defined($file) or next;
>  	if (m/^@@ .*?\+(\d+)/) { $line = $1; next }
>  	defined($line) or next;

The patch itself looks good. Nice and simple, and should not incur any extra cost or regress any other cases.

-Peff