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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 10, 2025, 13:20 UTC
Message-ID: <3f9eb0ed-576d-451a-93db-9b9508c99c27@gmail.com>
In-Reply-To: <cc90fefd-9234-4fb7-a00e-96c4004ddace@gmail.com>

On 10/08/2025 11:09, Phillip Wood wrote:

Show 11 quoted lines

> On 09/08/2025 15:44, D. Ben Knoble wrote:
>> On Fri, Aug 8, 2025 at 1:43 PM Greg Hurrell via GitGitGadget
>> <gitgitgadget@gmail.com> wrote:
>>> From: Greg Hurrell <greg.hurrell@datadoghq.com>
>>>
>>> This commit takes the simple approach of unconditionally stripping any
>>> trailing tab. Consider the following three examples:
>>>
>>> 1. For file "foo bar", Git will emit "foo bar\t".
>>> 2. For file "foo\t", Git will emit "foo\t".
>>> 3. For file "foo bar\t", Git will emit "foo bar\t\t".

When I wrote earlier I forgot that git quotes filenames with control characters. If a name contains a tab it it quoted and so cases 2 and 3 will be quoted and so there is no ambiguity when trimming a literal tab character from the end. I haven't checked but I suspect git-jump does not handle quoted filenames, if we wanted to add support it should be pretty easy as Git.pm has a function to do the unquoting for us.

Thanks

Phillip

Show 24 quoted lines

>>>
>>> Before this commit, `git-jump` correctly handled only case "2".
>>>
>>> After this commit, `git-jump` correctly handles cases "1" and "3". In
>>> reality, "1" is the only case people are going to run into with any
>>> regularity, and the other two are extreme edge cases.
>>
>> So we drop support for case 2? Hm. I personally try to avoid this
>> situation anyway, but it would be nice if we could just do the right
>> thing here.
>> Or maybe we should consider trying to parse --patch-with-raw output
>> for the filenames?
> 
> An alternative would be to parse the filename from the "diff --git" line 
> like "git apply" does. As we're generating the diff with "--no-prefix" 
> that should be straight forward as the line is "diff --git <name> 
> <name>" where <name> is the name of the post-image file unless it is a 
> deletion in which case it is the name of the pre-image file. We'd still 
> need to check the "+++ " line or look for a "deleted file mode" line to 
> handle deletions.
> 
> Thanks
> 
> Phillip