{"thread":{"id":"63931","subject":"[PATCH] git-jump: make `diff` work with filenames containing spaces","startedAt":"2025-08-08T17:43:03Z","lastAt":"2025-08-15T15:51:38Z","messageCount":11,"participants":["Greg Hurrell via GitGitGadget","D. Ben Knoble","Junio C Hamano","Phillip Wood","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"523833","messageId":"pull.1950.git.1754674979929.gitgitgadget@gmail.com","threadId":"63931","inReplyTo":null,"subject":"[PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"Greg Hurrell via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-08-08T17:42:59Z","receivedAt":"2025-08-08T17:43:03Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"From: Greg Hurrell <greg.hurrell@datadoghq.com>\n\nIn diff.c, we output a trailing \"\\t\" at the end of any filename that\ncontains a space:\n\n    case DIFF_SYMBOL_FILEPAIR_PLUS:\n            meta = diff_get_color_opt(o, DIFF_METAINFO);\n            reset = diff_get_color_opt(o, DIFF_RESET);\n            fprintf(o->file, \"%s%s+++ %s%s%s\\n\", diff_line_prefix(o), meta,\n                    line, reset,\n                    strchr(line, ' ') ? \"\\t\" : \"\");\n            break;\n\nThat is, for a file \"foo.txt\" we'll emit:\n\n    +++ a/foo.txt\n\nbut for \"foo bar.txt\" we'll emit:\n\n    +++ a/foo bar.txt\\t\n\nThis in turn leads us to produce a quickfix format like this:\n\n    foo bar.txt\\t:1:1:contents\n\nBecause no \"foo bar.txt\\t\" file actually exists on disk, opening it in\nVim will just land the user in an empty buffer.\n\nThis commit takes the simple approach of unconditionally stripping any\ntrailing tab. Consider the following three examples:\n\n1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n2. For file \"foo\\t\", Git will emit \"foo\\t\".\n3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n\nBefore this commit, `git-jump` correctly handled only case \"2\".\n\nAfter this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\nreality, \"1\" is the only case people are going to run into with any\nregularity, and the other two are extreme edge cases.\n\nThe argument here is that stripping the \"\\t\" unconditionally gives us a\nminimal change, and it addresses the common case without bringing in\ncomplexity for the uncommon ones. If anybody ever complains about case\n\"2\" no longer working for them, we can do the more complicated thing and\nonly strip the \"\\t\" if the filename contains a space.\n\nSigned-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n---\n    git-jump: make diff work with filenames containing spaces\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1950\n\n contrib/git-jump/git-jump | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 3f696759617..8d1d5d79a69 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -44,7 +44,7 @@ open_editor() {\n mode_diff() {\n \tgit diff --no-prefix --relative \"$@\" |\n \tperl -ne '\n-\tif (m{^\\+\\+\\+ (.*)}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n+\tif (m{^\\+\\+\\+ (.*?)\\t?$}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n \tdefined($file) or next;\n \tif (m/^@@ .*?\\+(\\d+)/) { $line = $1; next }\n \tdefined($line) or next;\n\nbase-commit: 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1\n-- \ngitgitgadget\n"},{"id":"523870","messageId":"CALnO6CDnSXpUVQEUJr=dc1ZY6errSv2M=4EmeaOmfDvcifHvnA@mail.gmail.com","threadId":"63931","inReplyTo":"pull.1950.git.1754674979929.gitgitgadget@gmail.com","subject":"Re: [PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-08-09T14:44:33Z","receivedAt":"2025-08-09T14:44:46Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Fri, Aug 8, 2025 at 1:43 PM Greg Hurrell via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: Greg Hurrell <greg.hurrell@datadoghq.com>\n>\n> In diff.c, we output a trailing \"\\t\" at the end of any filename that\n> contains a space:\n>\n>     case DIFF_SYMBOL_FILEPAIR_PLUS:\n>             meta = diff_get_color_opt(o, DIFF_METAINFO);\n>             reset = diff_get_color_opt(o, DIFF_RESET);\n>             fprintf(o->file, \"%s%s+++ %s%s%s\\n\", diff_line_prefix(o), meta,\n>                     line, reset,\n>                     strchr(line, ' ') ? \"\\t\" : \"\");\n>             break;\n>\n> That is, for a file \"foo.txt\" we'll emit:\n>\n>     +++ a/foo.txt\n>\n> but for \"foo bar.txt\" we'll emit:\n>\n>     +++ a/foo bar.txt\\t\n>\n\nA little spelunking dates this back to 1a9eb3b9d5 (git-diff/git-apply:\nmake diff output a bit friendlier to GNU patch (part 2), 2006-09-22),\nso we may be stuck with it :/\n\n> This in turn leads us to produce a quickfix format like this:\n>\n>     foo bar.txt\\t:1:1:contents\n>\n> Because no \"foo bar.txt\\t\" file actually exists on disk, opening it in\n> Vim will just land the user in an empty buffer.\n\nI can reproduce this with\n\n    echo 1 >'a b' && git add --intent-to-add a?b && git jump diff\n\n>\n> This commit takes the simple approach of unconditionally stripping any\n> trailing tab. Consider the following three examples:\n>\n> 1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n> 2. For file \"foo\\t\", Git will emit \"foo\\t\".\n> 3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n>\n> Before this commit, `git-jump` correctly handled only case \"2\".\n>\n> After this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\n> reality, \"1\" is the only case people are going to run into with any\n> regularity, and the other two are extreme edge cases.\n\nSo we drop support for case 2? Hm. I personally try to avoid this\nsituation anyway, but it would be nice if we could just do the right\nthing here.\nOr maybe we should consider trying to parse --patch-with-raw output\nfor the filenames?\n\n>\n> The argument here is that stripping the \"\\t\" unconditionally gives us a\n> minimal change, and it addresses the common case without bringing in\n> complexity for the uncommon ones. If anybody ever complains about case\n> \"2\" no longer working for them, we can do the more complicated thing and\n> only strip the \"\\t\" if the filename contains a space.\n>\n> Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n> ---\n>     git-jump: make diff work with filenames containing spaces\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1950\n>\n>  contrib/git-jump/git-jump | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> index 3f696759617..8d1d5d79a69 100755\n> --- a/contrib/git-jump/git-jump\n> +++ b/contrib/git-jump/git-jump\n> @@ -44,7 +44,7 @@ open_editor() {\n>  mode_diff() {\n>         git diff --no-prefix --relative \"$@\" |\n>         perl -ne '\n> -       if (m{^\\+\\+\\+ (.*)}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n> +       if (m{^\\+\\+\\+ (.*?)\\t?$}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n>         defined($file) or next;\n>         if (m/^@@ .*?\\+(\\d+)/) { $line = $1; next }\n>         defined($line) or next;\n>\n> base-commit: 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1\n> --\n> gitgitgadget\n>\n\nThis fix works as claimed and drops case (2) above, as discussed, so\nif we don't keep support for that then this looks right to me.\n\n\n-- \nD. Ben Knoble\n"},{"id":"523876","messageId":"xmqqjz3c59hd.fsf@gitster.g","threadId":"63931","inReplyTo":"pull.1950.git.1754674979929.gitgitgadget@gmail.com","subject":"Re: [PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-10T00:03:10Z","receivedAt":"2025-08-10T00:03:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Greg Hurrell via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Greg Hurrell <greg.hurrell@datadoghq.com>\n>\n> In diff.c, we output a trailing \"\\t\" at the end of any filename that\n> contains a space:\n>\n>     case DIFF_SYMBOL_FILEPAIR_PLUS:\n>             meta = diff_get_color_opt(o, DIFF_METAINFO);\n>             reset = diff_get_color_opt(o, DIFF_RESET);\n>             fprintf(o->file, \"%s%s+++ %s%s%s\\n\", diff_line_prefix(o), meta,\n>                     line, reset,\n>                     strchr(line, ' ') ? \"\\t\" : \"\");\n>             break;\n>\n> That is, for a file \"foo.txt\" we'll emit:\n>\n>     +++ a/foo.txt\n>\n> but for \"foo bar.txt\" we'll emit:\n>\n>     +++ a/foo bar.txt\\t\n>\n> This in turn leads us to produce a quickfix format like this:\n>\n>     foo bar.txt\\t:1:1:contents\n>\n> Because no \"foo bar.txt\\t\" file actually exists on disk, opening it in\n> Vim will just land the user in an empty buffer.\n>\n> This commit takes the simple approach of unconditionally stripping any\n> trailing tab. Consider the following three examples:\n>\n> 1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n> 2. For file \"foo\\t\", Git will emit \"foo\\t\".\n> 3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n>\n> Before this commit, `git-jump` correctly handled only case \"2\".\n>\n> After this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\n> reality, \"1\" is the only case people are going to run into with any\n> regularity, and the other two are extreme edge cases.\n>\n> The argument here is that stripping the \"\\t\" unconditionally gives us a\n> minimal change, and it addresses the common case without bringing in\n> complexity for the uncommon ones. If anybody ever complains about case\n> \"2\" no longer working for them, we can do the more complicated thing and\n> only strip the \"\\t\" if the filename contains a space.\n>\n> Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n> ---\n\nBecause (1) I do not use 'git jump', (2) I do not use 'vim' or\n'quickfix format', and (3) I know this is your brainchid but you are\noffline this week, I won't do anything to this topic other than\npossibly to keep it in 'seen' to avoid losing it.\n\nFWIW, I do not disagree with the decision of this patch makes to\n\"break\" those who has file \"foo\\t\" to help those with file \"foo\",\neven though I usually frown upon a change that robs Peter to pay\nPaul.  Among the three cases considerd, #1 is the only one that\nwould matter in practice.\n\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1950\n>\n>  contrib/git-jump/git-jump | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> index 3f696759617..8d1d5d79a69 100755\n> --- a/contrib/git-jump/git-jump\n> +++ b/contrib/git-jump/git-jump\n> @@ -44,7 +44,7 @@ open_editor() {\n>  mode_diff() {\n>  \tgit diff --no-prefix --relative \"$@\" |\n>  \tperl -ne '\n> -\tif (m{^\\+\\+\\+ (.*)}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n> +\tif (m{^\\+\\+\\+ (.*?)\\t?$}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n>  \tdefined($file) or next;\n>  \tif (m/^@@ .*?\\+(\\d+)/) { $line = $1; next }\n>  \tdefined($line) or next;\n>\n> base-commit: 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1\n"},{"id":"523883","messageId":"cc90fefd-9234-4fb7-a00e-96c4004ddace@gmail.com","threadId":"63931","inReplyTo":"CALnO6CDnSXpUVQEUJr=dc1ZY6errSv2M=4EmeaOmfDvcifHvnA@mail.gmail.com","subject":"Re: [PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-08-10T10:09:19Z","receivedAt":"2025-08-10T10:09:00Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 09/08/2025 15:44, D. Ben Knoble wrote:\n> On Fri, Aug 8, 2025 at 1:43 PM Greg Hurrell via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n>> From: Greg Hurrell <greg.hurrell@datadoghq.com>\n>>\n>> This commit takes the simple approach of unconditionally stripping any\n>> trailing tab. Consider the following three examples:\n>>\n>> 1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n>> 2. For file \"foo\\t\", Git will emit \"foo\\t\".\n>> 3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n>>\n>> Before this commit, `git-jump` correctly handled only case \"2\".\n>>\n>> After this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\n>> reality, \"1\" is the only case people are going to run into with any\n>> regularity, and the other two are extreme edge cases.\n> \n> So we drop support for case 2? Hm. I personally try to avoid this\n> situation anyway, but it would be nice if we could just do the right\n> thing here.\n> Or maybe we should consider trying to parse --patch-with-raw output\n> for the filenames?\n\nAn alternative would be to parse the filename from the \"diff --git\" line \nlike \"git apply\" does. As we're generating the diff with \"--no-prefix\" \nthat should be straight forward as the line is \"diff --git <name> \n<name>\" where <name> is the name of the post-image file unless it is a \ndeletion in which case it is the name of the pre-image file. We'd still \nneed to check the \"+++ \" line or look for a \"deleted file mode\" line to \nhandle deletions.\n\nThanks\n\nPhillip\n"},{"id":"523884","messageId":"3f9eb0ed-576d-451a-93db-9b9508c99c27@gmail.com","threadId":"63931","inReplyTo":"cc90fefd-9234-4fb7-a00e-96c4004ddace@gmail.com","subject":"Re: [PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-08-10T13:20:13Z","receivedAt":"2025-08-10T13:19:54Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 10/08/2025 11:09, Phillip Wood wrote:\n> On 09/08/2025 15:44, D. Ben Knoble wrote:\n>> On Fri, Aug 8, 2025 at 1:43 PM Greg Hurrell via GitGitGadget\n>> <gitgitgadget@gmail.com> wrote:\n>>> From: Greg Hurrell <greg.hurrell@datadoghq.com>\n>>>\n>>> This commit takes the simple approach of unconditionally stripping any\n>>> trailing tab. Consider the following three examples:\n>>>\n>>> 1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n>>> 2. For file \"foo\\t\", Git will emit \"foo\\t\".\n>>> 3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n\nWhen I wrote earlier I forgot that git quotes filenames with control \ncharacters. If a name contains a tab it it quoted and so cases 2 and 3 \nwill be quoted and so there is no ambiguity when trimming a literal tab \ncharacter from the end. I haven't checked but I suspect git-jump does \nnot handle quoted filenames, if we wanted to add support it should be \npretty easy as Git.pm has a function to do the unquoting for us.\n\nThanks\n\nPhillip\n\n>>>\n>>> Before this commit, `git-jump` correctly handled only case \"2\".\n>>>\n>>> After this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\n>>> reality, \"1\" is the only case people are going to run into with any\n>>> regularity, and the other two are extreme edge cases.\n>>\n>> So we drop support for case 2? Hm. I personally try to avoid this\n>> situation anyway, but it would be nice if we could just do the right\n>> thing here.\n>> Or maybe we should consider trying to parse --patch-with-raw output\n>> for the filenames?\n> \n> An alternative would be to parse the filename from the \"diff --git\" line \n> like \"git apply\" does. As we're generating the diff with \"--no-prefix\" \n> that should be straight forward as the line is \"diff --git <name> \n> <name>\" where <name> is the name of the post-image file unless it is a \n> deletion in which case it is the name of the pre-image file. We'd still \n> need to check the \"+++ \" line or look for a \"deleted file mode\" line to \n> handle deletions.\n> \n> Thanks\n> \n> Phillip\n\n"},{"id":"523923","messageId":"pull.1950.v2.git.1754913323810.gitgitgadget@gmail.com","threadId":"63931","inReplyTo":"pull.1950.git.1754674979929.gitgitgadget@gmail.com","subject":"[PATCH v2] git-jump: make `diff` work with filenames containing spaces","fromName":"Greg Hurrell via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-08-11T11:55:23Z","receivedAt":"2025-08-11T11:55:27Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"From: Greg Hurrell <greg.hurrell@datadoghq.com>\n\nIn diff.c, we output a trailing \"\\t\" at the end of any filename that\ncontains a space:\n\n    case DIFF_SYMBOL_FILEPAIR_PLUS:\n            meta = diff_get_color_opt(o, DIFF_METAINFO);\n            reset = diff_get_color_opt(o, DIFF_RESET);\n            fprintf(o->file, \"%s%s+++ %s%s%s\\n\", diff_line_prefix(o), meta,\n                    line, reset,\n                    strchr(line, ' ') ? \"\\t\" : \"\");\n            break;\n\nThat is, for a file \"foo.txt\", `git diff --no-prefix` will emit:\n\n    +++ foo.txt\n\nbut for \"foo bar.txt\" it will emit:\n\n    +++ foo bar.txt\\t\n\nThis in turn leads `git-jump` to produce a quickfix format like this:\n\n    foo bar.txt\\t:1:1:contents\n\nBecause no \"foo bar.txt\\t\" file actually exists on disk, opening it in\nVim will just land the user in an empty buffer.\n\nThis commit takes the simple approach of unconditionally stripping any\ntrailing tab. Consider the following three examples:\n\n1. For file \"foo\", Git will emit \"foo\".\n2. For file \"foo bar\", Git will emit \"foo bar\\t\".\n3. For file \"foo\\t\", Git will emit \"\\\"foo\\t\\\"\".\n4. For file \"foo bar\\t\", Git will emit \"\\\"foo bar\\t\\\"\".\n\nBefore this commit, `git-jump` correctly handled only case \"1\".\n\nAfter this commit, `git-jump` correctly handles cases \"1\" and \"2\". In\nreality, these are the only cases people are going to run into with any\nregularity, and the other two are rare edge cases, which probably aren't\nworth the effort to support unless somebody actually complains about\nthem.\n\nSigned-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n---\n    git-jump: make diff work with filenames containing spaces\n    \n    Changed since v1:\n    \n     * No code changes, but reworded commit message to include examples of\n       quoted paths.\n    \n    Turns out that quoted paths never worked, so this commit isn't \"robbing\n    Peter to pay Paul\", but rather, \"giving something to Paul for free\n    (Peter, sadly, is still out of luck)\".\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1950\n\nRange-diff vs v1:\n\n 1:  afe01c156e5 ! 1:  03fa9ac1ab2 git-jump: make `diff` work with filenames containing spaces\n     @@ Commit message\n                              strchr(line, ' ') ? \"\\t\" : \"\");\n                      break;\n      \n     -    That is, for a file \"foo.txt\" we'll emit:\n     +    That is, for a file \"foo.txt\", `git diff --no-prefix` will emit:\n      \n     -        +++ a/foo.txt\n     +        +++ foo.txt\n      \n     -    but for \"foo bar.txt\" we'll emit:\n     +    but for \"foo bar.txt\" it will emit:\n      \n     -        +++ a/foo bar.txt\\t\n     +        +++ foo bar.txt\\t\n      \n     -    This in turn leads us to produce a quickfix format like this:\n     +    This in turn leads `git-jump` to produce a quickfix format like this:\n      \n              foo bar.txt\\t:1:1:contents\n      \n     @@ Commit message\n          This commit takes the simple approach of unconditionally stripping any\n          trailing tab. Consider the following three examples:\n      \n     -    1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n     -    2. For file \"foo\\t\", Git will emit \"foo\\t\".\n     -    3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n     +    1. For file \"foo\", Git will emit \"foo\".\n     +    2. For file \"foo bar\", Git will emit \"foo bar\\t\".\n     +    3. For file \"foo\\t\", Git will emit \"\\\"foo\\t\\\"\".\n     +    4. For file \"foo bar\\t\", Git will emit \"\\\"foo bar\\t\\\"\".\n      \n     -    Before this commit, `git-jump` correctly handled only case \"2\".\n     +    Before this commit, `git-jump` correctly handled only case \"1\".\n      \n     -    After this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\n     -    reality, \"1\" is the only case people are going to run into with any\n     -    regularity, and the other two are extreme edge cases.\n     -\n     -    The argument here is that stripping the \"\\t\" unconditionally gives us a\n     -    minimal change, and it addresses the common case without bringing in\n     -    complexity for the uncommon ones. If anybody ever complains about case\n     -    \"2\" no longer working for them, we can do the more complicated thing and\n     -    only strip the \"\\t\" if the filename contains a space.\n     +    After this commit, `git-jump` correctly handles cases \"1\" and \"2\". In\n     +    reality, these are the only cases people are going to run into with any\n     +    regularity, and the other two are rare edge cases, which probably aren't\n     +    worth the effort to support unless somebody actually complains about\n     +    them.\n      \n          Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n      \n\n\n contrib/git-jump/git-jump | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 3f696759617..8d1d5d79a69 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -44,7 +44,7 @@ open_editor() {\n mode_diff() {\n \tgit diff --no-prefix --relative \"$@\" |\n \tperl -ne '\n-\tif (m{^\\+\\+\\+ (.*)}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n+\tif (m{^\\+\\+\\+ (.*?)\\t?$}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n \tdefined($file) or next;\n \tif (m/^@@ .*?\\+(\\d+)/) { $line = $1; next }\n \tdefined($line) or next;\n\nbase-commit: 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1\n-- \ngitgitgadget\n"},{"id":"523932","messageId":"4e2e2bea-c8e5-4343-9e70-a2bd139eb242@gmail.com","threadId":"63931","inReplyTo":"pull.1950.v2.git.1754913323810.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git-jump: make `diff` work with filenames containing spaces","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-08-11T13:16:11Z","receivedAt":"2025-08-11T13:15:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Greg\n\nOn 11/08/2025 12:55, Greg Hurrell via GitGitGadget wrote:\n> From: Greg Hurrell <greg.hurrell@datadoghq.com>\n> [...]\n> 1. For file \"foo\", Git will emit \"foo\".\n> 2. For file \"foo bar\", Git will emit \"foo bar\\t\".\n> 3. For file \"foo\\t\", Git will emit \"\\\"foo\\t\\\"\".\n> 4. For file \"foo bar\\t\", Git will emit \"\\\"foo bar\\t\\\"\".\n> \n> Before this commit, `git-jump` correctly handled only case \"1\".\n> \n> After this commit, `git-jump` correctly handles cases \"1\" and \"2\". In\n> reality, these are the only cases people are going to run into with any\n> regularity, and the other two are rare edge cases, which probably aren't\n> worth the effort to support unless somebody actually complains about\n> them.\n\nThanks for updating the commit message, I agree it's probably not worth \nworrying about cases 3 & 4 unless someone complains\n\nThanks\n\nPhillip\n\n> Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n> ---\n>      git-jump: make diff work with filenames containing spaces\n>      \n>      Changed since v1:\n>      \n>       * No code changes, but reworded commit message to include examples of\n>         quoted paths.\n>      \n>      Turns out that quoted paths never worked, so this commit isn't \"robbing\n>      Peter to pay Paul\", but rather, \"giving something to Paul for free\n>      (Peter, sadly, is still out of luck)\".\n> \n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1950%2Fwincent%2Fstrip-trailing-tab-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1950/wincent/strip-trailing-tab-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1950\n> \n> Range-diff vs v1:\n> \n>   1:  afe01c156e5 ! 1:  03fa9ac1ab2 git-jump: make `diff` work with filenames containing spaces\n>       @@ Commit message\n>                                strchr(line, ' ') ? \"\\t\" : \"\");\n>                        break;\n>        \n>       -    That is, for a file \"foo.txt\" we'll emit:\n>       +    That is, for a file \"foo.txt\", `git diff --no-prefix` will emit:\n>        \n>       -        +++ a/foo.txt\n>       +        +++ foo.txt\n>        \n>       -    but for \"foo bar.txt\" we'll emit:\n>       +    but for \"foo bar.txt\" it will emit:\n>        \n>       -        +++ a/foo bar.txt\\t\n>       +        +++ foo bar.txt\\t\n>        \n>       -    This in turn leads us to produce a quickfix format like this:\n>       +    This in turn leads `git-jump` to produce a quickfix format like this:\n>        \n>                foo bar.txt\\t:1:1:contents\n>        \n>       @@ Commit message\n>            This commit takes the simple approach of unconditionally stripping any\n>            trailing tab. Consider the following three examples:\n>        \n>       -    1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n>       -    2. For file \"foo\\t\", Git will emit \"foo\\t\".\n>       -    3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n>       +    1. For file \"foo\", Git will emit \"foo\".\n>       +    2. For file \"foo bar\", Git will emit \"foo bar\\t\".\n>       +    3. For file \"foo\\t\", Git will emit \"\\\"foo\\t\\\"\".\n>       +    4. For file \"foo bar\\t\", Git will emit \"\\\"foo bar\\t\\\"\".\n>        \n>       -    Before this commit, `git-jump` correctly handled only case \"2\".\n>       +    Before this commit, `git-jump` correctly handled only case \"1\".\n>        \n>       -    After this commit, `git-jump` correctly handles cases \"1\" and \"3\". In\n>       -    reality, \"1\" is the only case people are going to run into with any\n>       -    regularity, and the other two are extreme edge cases.\n>       -\n>       -    The argument here is that stripping the \"\\t\" unconditionally gives us a\n>       -    minimal change, and it addresses the common case without bringing in\n>       -    complexity for the uncommon ones. If anybody ever complains about case\n>       -    \"2\" no longer working for them, we can do the more complicated thing and\n>       -    only strip the \"\\t\" if the filename contains a space.\n>       +    After this commit, `git-jump` correctly handles cases \"1\" and \"2\". In\n>       +    reality, these are the only cases people are going to run into with any\n>       +    regularity, and the other two are rare edge cases, which probably aren't\n>       +    worth the effort to support unless somebody actually complains about\n>       +    them.\n>        \n>            Signed-off-by: Greg Hurrell <greg.hurrell@datadoghq.com>\n>        \n> \n> \n>   contrib/git-jump/git-jump | 2 +-\n>   1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> index 3f696759617..8d1d5d79a69 100755\n> --- a/contrib/git-jump/git-jump\n> +++ b/contrib/git-jump/git-jump\n> @@ -44,7 +44,7 @@ open_editor() {\n>   mode_diff() {\n>   \tgit diff --no-prefix --relative \"$@\" |\n>   \tperl -ne '\n> -\tif (m{^\\+\\+\\+ (.*)}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n> +\tif (m{^\\+\\+\\+ (.*?)\\t?$}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n>   \tdefined($file) or next;\n>   \tif (m/^@@ .*?\\+(\\d+)/) { $line = $1; next }\n>   \tdefined($line) or next;\n> \n> base-commit: 2c2ba49d55ff26c1082b8137b1ec5eeccb4337d1\n\n"},{"id":"523982","messageId":"CALnO6CChgchH-KPyNwwy9zf41c_2miqza4rWS3NxzpZFcJmEsg@mail.gmail.com","threadId":"63931","inReplyTo":"4e2e2bea-c8e5-4343-9e70-a2bd139eb242@gmail.com","subject":"Re: [PATCH v2] git-jump: make `diff` work with filenames containing spaces","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-08-11T21:05:43Z","receivedAt":"2025-08-11T21:05:57Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Aug 11, 2025 at 9:15 AM Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> Hi Greg\n>\n> On 11/08/2025 12:55, Greg Hurrell via GitGitGadget wrote:\n> > From: Greg Hurrell <greg.hurrell@datadoghq.com>\n> > [...]\n> > 1. For file \"foo\", Git will emit \"foo\".\n> > 2. For file \"foo bar\", Git will emit \"foo bar\\t\".\n> > 3. For file \"foo\\t\", Git will emit \"\\\"foo\\t\\\"\".\n> > 4. For file \"foo bar\\t\", Git will emit \"\\\"foo bar\\t\\\"\".\n> >\n> > Before this commit, `git-jump` correctly handled only case \"1\".\n> >\n> > After this commit, `git-jump` correctly handles cases \"1\" and \"2\". In\n> > reality, these are the only cases people are going to run into with any\n> > regularity, and the other two are rare edge cases, which probably aren't\n> > worth the effort to support unless somebody actually complains about\n> > them.\n>\n> Thanks for updating the commit message, I agree it's probably not worth\n> worrying about cases 3 & 4 unless someone complains\n>\n> Thanks\n>\n> Phillip\n\nAgreed, and fine by me (since we have a strict improvement).\n"},{"id":"524197","messageId":"20250814231849.GB2937@coredump.intra.peff.net","threadId":"63931","inReplyTo":"pull.1950.v2.git.1754913323810.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] git-jump: make `diff` work with filenames containing spaces","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-08-14T23:18:49Z","receivedAt":"2025-08-14T23:18:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 11, 2025 at 11:55:23AM +0000, Greg Hurrell via GitGitGadget wrote:\n\n> This commit takes the simple approach of unconditionally stripping any\n> trailing tab. Consider the following three examples:\n> \n> 1. For file \"foo\", Git will emit \"foo\".\n> 2. For file \"foo bar\", Git will emit \"foo bar\\t\".\n> 3. For file \"foo\\t\", Git will emit \"\\\"foo\\t\\\"\".\n> 4. For file \"foo bar\\t\", Git will emit \"\\\"foo bar\\t\\\"\".\n> \n> Before this commit, `git-jump` correctly handled only case \"1\".\n> \n> After this commit, `git-jump` correctly handles cases \"1\" and \"2\". In\n> reality, these are the only cases people are going to run into with any\n> regularity, and the other two are rare edge cases, which probably aren't\n> worth the effort to support unless somebody actually complains about\n> them.\n\nThanks for laying out these cases. I think this list shows that we are\nmaking things strictly better, but just stopping short of handling\nunquoting. So it seems like a no-brainer to take this patch (though I\nthink even with the description in v1, in which we thought we might\nregress \"foo\\t\", I think it would still have been worth it).\n\nAnd I think this is a good stopping point. Handling unquoting would be\ntricky for a case that is unlikely to come up in practice. And I'm not\neven sure we could make it foolproof, anyway. We're feeding this to the\neditor's quickfix parser, so I'm not sure how we'd represent something\nlike a newline. We'd have to agree on the quoting scheme with the\neditor, and from my (admittedly brief) research, there is not even a\nmechanism in vim for that.\n\n> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> index 3f696759617..8d1d5d79a69 100755\n> --- a/contrib/git-jump/git-jump\n> +++ b/contrib/git-jump/git-jump\n> @@ -44,7 +44,7 @@ open_editor() {\n>  mode_diff() {\n>  \tgit diff --no-prefix --relative \"$@\" |\n>  \tperl -ne '\n> -\tif (m{^\\+\\+\\+ (.*)}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n> +\tif (m{^\\+\\+\\+ (.*?)\\t?$}) { $file = $1 eq \"/dev/null\" ? undef : $1; next }\n>  \tdefined($file) or next;\n>  \tif (m/^@@ .*?\\+(\\d+)/) { $line = $1; next }\n>  \tdefined($line) or next;\n\nThe patch itself looks good. Nice and simple, and should not incur any\nextra cost or regress any other cases.\n\n-Peff\n"},{"id":"524198","messageId":"20250814231402.GA2937@coredump.intra.peff.net","threadId":"63931","inReplyTo":"3f9eb0ed-576d-451a-93db-9b9508c99c27@gmail.com","subject":"Re: [PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-08-14T23:14:02Z","receivedAt":"2025-08-14T23:20:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 10, 2025 at 02:20:13PM +0100, Phillip Wood wrote:\n\n> On 10/08/2025 11:09, Phillip Wood wrote:\n> > On 09/08/2025 15:44, D. Ben Knoble wrote:\n> > > On Fri, Aug 8, 2025 at 1:43 PM Greg Hurrell via GitGitGadget\n> > > <gitgitgadget@gmail.com> wrote:\n> > > > From: Greg Hurrell <greg.hurrell@datadoghq.com>\n> > > > \n> > > > This commit takes the simple approach of unconditionally stripping any\n> > > > trailing tab. Consider the following three examples:\n> > > > \n> > > > 1. For file \"foo bar\", Git will emit \"foo bar\\t\".\n> > > > 2. For file \"foo\\t\", Git will emit \"foo\\t\".\n> > > > 3. For file \"foo bar\\t\", Git will emit \"foo bar\\t\\t\".\n> \n> When I wrote earlier I forgot that git quotes filenames with control\n> characters. If a name contains a tab it it quoted and so cases 2 and 3 will\n> be quoted and so there is no ambiguity when trimming a literal tab character\n> from the end. I haven't checked but I suspect git-jump does not handle\n> quoted filenames, if we wanted to add support it should be pretty easy as\n> Git.pm has a function to do the unquoting for us.\n\nYeah, git-jump does not do any unquoting at all. Ironically I used the\n\"+++\" line because I wanted to avoid quoting and whitespace headaches on\nthe \"diff --git\" line. But I guess it is unavoidable for truly weird\npath names. ;)\n\nI'd prefer to avoid an extra dependency on Git.pm and just leave it\nbroken for quoted names. Since names with spaces are the likely thing to\nsee, and those aren't quoted, I think running into this should be pretty\nrare (another alternative is to lazy-load Git.pm only when necessary,\nsince we're already in a perl script).\n\nI wondered if it might almost work without any intelligence on the part\nof git-jump, just because vim's quickfix parser already does a bunch of\nheuristic regex matches, some of which understand quotes. But it looks\nlike the answer is no. This one from the default set:\n\n  \"%f\"%*\\D%l: %m\n\nis quite close, and would match:\n\n  echo content >'foo bar'\n  vim -q <(printf '\"foo bar\":1: some error')\n\nbut it won't automatically undo backslash escapes inside the quoted\nportion. So in something that actually needed quoting would end up\nlooking for a file with the literal sequence \"\\t\" in it, rather than a\ntab. Oh well.\n\n-Peff\n"},{"id":"524255","messageId":"101c0157-a542-48e3-941d-d4c84fe2efc1@gmail.com","threadId":"63931","inReplyTo":"20250814231402.GA2937@coredump.intra.peff.net","subject":"Re: [PATCH] git-jump: make `diff` work with filenames containing spaces","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-08-15T15:51:36Z","receivedAt":"2025-08-15T15:51:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Peff\n\nOn 15/08/2025 00:14, Jeff King wrote:\n> On Sun, Aug 10, 2025 at 02:20:13PM +0100, Phillip Wood wrote:\n>> On 10/08/2025 11:09, Phillip Wood wrote:\n>> \n>> When I wrote earlier I forgot that git quotes filenames with control\n>> characters. If a name contains a tab it it quoted and so cases 2 and 3 will\n>> be quoted and so there is no ambiguity when trimming a literal tab character\n>> from the end. I haven't checked but I suspect git-jump does not handle\n>> quoted filenames, if we wanted to add support it should be pretty easy as\n>> Git.pm has a function to do the unquoting for us.\n> \n> Yeah, git-jump does not do any unquoting at all. Ironically I used the\n> \"+++\" line because I wanted to avoid quoting and whitespace headaches on\n> the \"diff --git\" line. But I guess it is unavoidable for truly weird\n> path names. ;)\n> \n> I'd prefer to avoid an extra dependency on Git.pm and just leave it\n> broken for quoted names. Since names with spaces are the likely thing to\n> see, and those aren't quoted, I think running into this should be pretty\n> rare (another alternative is to lazy-load Git.pm only when necessary,\n> since we're already in a perl script).\n\nI agree there's no pressing need to handle quoted names.\n\nThanks\n\nPhillip\n\n"}]}