Re: [PATCH v2] git-jump: make `diff` work with filenames containing spaces
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Aug 11, 2025, 13:16 UTC
- Message-ID
- <4e2e2bea-c8e5-4343-9e70-a2bd139eb242@gmail.com>
- In-Reply-To
- <pull.1950.v2.git.1754913323810.gitgitgadget@gmail.com>
Hi Greg
On 11/08/2025 12:55, Greg Hurrell via GitGitGadget wrote:
Show 14 quoted lines
> From: Greg Hurrell <greg.hurrell@datadoghq.com> > [...] > 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 updating the commit message, I agree it's probably not worth worrying about cases 3 & 4 unless someone complains
Thanks
Phillip
Show 93 quoted lines
> Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>
> ---
> git-jump: make diff work with filenames containing spaces
>
> Changed since v1:
>
> * No code changes, but reworded commit message to include examples of
> quoted paths.
>
> Turns out that quoted paths never worked, so this commit isn't "robbing
> Peter to pay Paul", but rather, "giving something to Paul for free
> (Peter, sadly, is still out of luck)".
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1950
>
> Range-diff vs v1:
>
> 1: afe01c156e5 ! 1: 03fa9ac1ab2 git-jump: make `diff` work with filenames containing spaces
> @@ Commit message
> strchr(line, ' ') ? "\t" : "");
> break;
>
> - That is, for a file "foo.txt" we'll emit:
> + That is, for a file "foo.txt", `git diff --no-prefix` will emit:
>
> - +++ a/foo.txt
> + +++ foo.txt
>
> - but for "foo bar.txt" we'll emit:
> + but for "foo bar.txt" it will emit:
>
> - +++ a/foo bar.txt\t
> + +++ foo bar.txt\t
>
> - This in turn leads us to produce a quickfix format like this:
> + This in turn leads `git-jump` to produce a quickfix format like this:
>
> foo bar.txt\t:1:1:contents
>
> @@ Commit message
> 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".
> + 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 "2".
> + Before this commit, `git-jump` correctly handled only case "1".
>
> - 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.
> -
> - The argument here is that stripping the "\t" unconditionally gives us a
> - minimal change, and it addresses the common case without bringing in
> - complexity for the uncommon ones. If anybody ever complains about case
> - "2" no longer working for them, we can do the more complicated thing and
> - only strip the "\t" if the filename contains a space.
> + 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.
>
> Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>
>
>
>
> contrib/git-jump/git-jump | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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;
>
> base-commit: 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1