git-jump: make `diff` work with filenames containing spaces

From: Greg Hurrell <[email protected]>

In diff.c, we output a trailing "\t" at the end of any filename that contains a space:

    case DIFF_SYMBOL_FILEPAIR_PLUS:
            meta = diff_get_color_opt(o, DIFF_METAINFO);
            reset = diff_get_color_opt(o, DIFF_RESET);
            fprintf(o->file, "%s%s+++ %s%s%s\n", diff_line_prefix(o), meta,
                    line, reset,
                    strchr(line, ' ') ? "\t" : "");
            break;

That is, for a file "foo.txt" we'll emit:

    +++ a/foo.txt

but for "foo bar.txt" we'll emit:

    +++ a/foo bar.txt\t

This in turn leads us to produce a quickfix format like this:

    foo bar.txt\t:1:1:contents

Because no "foo bar.txt\t" file actually exists on disk, opening it in Vim will just land the user in an empty buffer.

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".

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.

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.

Signed-off-by Greg Hurrell <[email protected]>

---
git-jump: make diff work with filenames containing spaces

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1950

 contrib/git-jump/git-jump | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

contrib/git-jump/git-jump +1 -1

index 3f696759617..8d1d5d79a69 100755
@@ -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 --
gitgitgadget

From: Greg Hurrell <[email protected]>

In diff.c, we output a trailing "\t" at the end of any filename that contains a space:

    case DIFF_SYMBOL_FILEPAIR_PLUS:
            meta = diff_get_color_opt(o, DIFF_METAINFO);
            reset = diff_get_color_opt(o, DIFF_RESET);
            fprintf(o->file, "%s%s+++ %s%s%s\n", diff_line_prefix(o), meta,
                    line, reset,
                    strchr(line, ' ') ? "\t" : "");
            break;

That is, for a file "foo.txt" we'll emit:

    +++ a/foo.txt

but for "foo bar.txt" we'll emit:

    +++ a/foo bar.txt\t

This in turn leads us to produce a quickfix format like this:

    foo bar.txt\t:1:1:contents

Because no "foo bar.txt\t" file actually exists on disk, opening it in Vim will just land the user in an empty buffer.

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".

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.

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.

Signed-off-by Greg Hurrell <[email protected]>

---

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1950

 contrib/git-jump/git-jump | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

contrib/git-jump/git-jump +1 -1

index 3f696759617..8d1d5d79a69 100755
@@ -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

Quoted 9 lines · Greg Hurrell via GitGitGadget

From: Greg Hurrell <[email protected]>

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".

Quoted 5 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?

Quoted 10 lines · D. Ben Knoble

Quoted 8 lines · Greg Hurrell via GitGitGadget

From: Greg Hurrell <[email protected]>

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".

Quoted 7 lines

Quoted 6 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.

Quoted 5 lines

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

From: Greg Hurrell <[email protected]> [...]
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.

Signed-off-by Greg Hurrell <[email protected]>

---
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 <[email protected]>

  contrib/git-jump/git-jump | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

contrib/git-jump/git-jump +1 -1

index 3f696759617..8d1d5d79a69 100755
@@ -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

Hi Greg

Quoted 7 lines · Greg Hurrell via GitGitGadget

From: Greg Hurrell <[email protected]> [...]
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\"".

Quoted 7 lines

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

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.

Quoted 11 lines · Phillip Wood

Quoted 10 lines · D. Ben Knoble

Quoted 8 lines · Greg Hurrell via GitGitGadget

From: Greg Hurrell <[email protected]>

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.

Quoted 8 lines · Phillip Wood

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

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.

Yeah, git-jump does not do any unquoting at all. Ironically I used the "+++" line because I wanted to avoid quoting and whitespace headaches on the "diff --git" line. But I guess it is unavoidable for truly weird path names. ;)

I'd prefer to avoid an extra dependency on Git.pm and just leave it broken for quoted names. Since names with spaces are the likely thing to see, and those aren't quoted, I think running into this should be pretty rare (another alternative is to lazy-load Git.pm only when necessary, since we're already in a perl script).