{"thread":{"id":"54206","subject":"[Bug report] git status doesn't escape paths of untracked files","startedAt":"2020-09-08T00:29:34Z","lastAt":"2020-09-10T23:03:32Z","messageCount":41,"participants":["Patrick Fong","Junio C Hamano","brian m. carlson","Martin Ågren","René Scharfe","Chris Torek","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"405142","messageId":"CAMRL+qb0YC1EOTM-LDfMpJ=AJJ014LT5ufBcs0v77byN74A0vw@mail.gmail.com","threadId":"54206","inReplyTo":null,"subject":"[Bug report] git status doesn't escape paths of untracked files","fromName":"Patrick Fong","fromEmail":"patrickf3139@gmail.com","sentAt":"2020-09-08T00:28:57Z","receivedAt":"2020-09-08T00:29:34Z","isPatch":false,"sender":{"key":"patrickf3139@gmail.com","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\ntouch \"this is escaped\"\ntouch \"this is not escaped\"\ngit add \"this is escaped\"\ngit status --short\n\nWhat did you expect to happen? (Expected behavior)\nI expected that git status --short would string escape both \"this is\nescaped\" and \"this is not escaped\" since they both contain spaces that\nneed escaping.\n\nWhat happened instead? (Actual behavior)\ngit status did not string escape \"this is not escaped\" but it does if\nyou add it to the index.\n\nWhat's different between what you expected and what actually happened?\n\nAnything else you want to add:\nOnce you add \"this is not escaped\", git status will output it with\nquotes. git status --short seems to change its behavior of escaping\nbased on whether the path is tracked or untracked.\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.28.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nuname: Darwin 19.5.0 Darwin Kernel Version 19.5.0: Tue May 26 20:41:44\nPDT 2020; root:xnu-6153.121.2~2/RELEASE_X86_64 x86_64\ncompiler info: clang: 11.0.3 (clang-1103.0.32.62)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /usr/local/bin/fish\n\n\n[Enabled Hooks]\n"},{"id":"405143","messageId":"xmqqa6y1135o.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"CAMRL+qb0YC1EOTM-LDfMpJ=AJJ014LT5ufBcs0v77byN74A0vw@mail.gmail.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T01:13:39Z","receivedAt":"2020-09-08T01:13:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Fong writes:\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> touch \"this is escaped\"\n> touch \"this is not escaped\"\n> git add \"this is escaped\"\n> git status --short\n>\n> What did you expect to happen? (Expected behavior)\n> I expected that git status --short would string escape both \"this is\n> escaped\" and \"this is not escaped\" since they both contain spaces that\n> need escaping.\n>\n> What happened instead? (Actual behavior)\n> git status did not string escape \"this is not escaped\" but it does if\n> you add it to the index.\n\nIt indeed is a disturbing inconsistency, and the code that shows the\nuntracked paths should be fixed. I wonder what is done to ignored\npaths when the \"--ignored\" option is given.\n\nwt-status.c::wt_shortstatus_status() has a strange special case that\nsays \"whitespace alone does not usually deserve quoting, but we\nspecial case it and manually quote\"; this discrepancy/special casing\nwas introduced at dbfdc625 (status: Quote paths with spaces in short\nformat, 2010-11-08). A fix would probably be to port the same fix\nto wt-status.c::wt_shortstatus_other().\n\nIn the longer term, quote.c::quote_path() may want to learn an\noptional flag to tell it to do the the special casing.\n\nThanks. \n"},{"id":"405144","messageId":"20200908011756.GG241078@camp.crustytoothpaste.net","threadId":"54206","inReplyTo":"CAMRL+qb0YC1EOTM-LDfMpJ=AJJ014LT5ufBcs0v77byN74A0vw@mail.gmail.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-09-08T01:17:56Z","receivedAt":"2020-09-08T01:18:36Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-09-08 at 00:28:57, Patrick Fong wrote:\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> touch \"this is escaped\"\n> touch \"this is not escaped\"\n> git add \"this is escaped\"\n> git status --short\n> \n> What did you expect to happen? (Expected behavior)\n> I expected that git status --short would string escape both \"this is\n> escaped\" and \"this is not escaped\" since they both contain spaces that\n> need escaping.\n> \n> What happened instead? (Actual behavior)\n> git status did not string escape \"this is not escaped\" but it does if\n> you add it to the index.\n> \n> What's different between what you expected and what actually happened?\n> \n> Anything else you want to add:\n> Once you add \"this is not escaped\", git status will output it with\n> quotes. git status --short seems to change its behavior of escaping\n> based on whether the path is tracked or untracked.\n\ngit-status(1) says:\n\n If a filename contains whitespace or other nonprintable characters,\n that field will be quoted in the manner of a C string literal:\n surrounded by ASCII double quote (34) characters, and with interior\n special characters backslash-escaped.\n\nNote that that differs from the standard behavior of not handling\nspaces, which I expect is due to the need to handle renames\nunambiguously. We have some special handling in wt_shortstatus_status,\nbut not in wt_shortstatus_other.\n\nPatch incoming once the testsuite finishes running on this commit.\n-- \nbrian m. carlson: Houston, Texas, US\n"},{"id":"405145","messageId":"xmqq5z8p12ds.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"20200908011756.GG241078@camp.crustytoothpaste.net","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T01:30:23Z","receivedAt":"2020-09-08T01:30:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> git-status(1) says:\n>\n> If a filename contains whitespace or other nonprintable characters,\n> that field will be quoted in the manner of a C string literal:\n> surrounded by ASCII double quote (34) characters, and with interior\n> special characters backslash-escaped.\n>\n> Note that that differs from the standard behavior of not handling\n> spaces, which I expect is due to the need to handle renames\n> unambiguously.\n\nNot really. We use \"rename from\" and \"rename to\" extended header\nlines in our output to unambiguously handle renamed paths.\n\ncquote was indeed invented to serve \"diff\" output (actually, nice\nthings in the internal implementation of Git from the early years\nwere all invented to serve the diff machinery), but we deliberately\nexcluded SP from the set of characters that needs quoting because\nit was thought to be reasonably common, compared to things like HT.\n\nI agree with your \"the special case handling needs to be taught to\nthe wt_shortstatus_other()\"; a refactored helper function called\nby both places would help.\n\nThanks.\n\n"},{"id":"405146","messageId":"20200908013013.1099937-1-sandals@crustytoothpaste.net","threadId":"54206","inReplyTo":"CAMRL+qb0YC1EOTM-LDfMpJ=AJJ014LT5ufBcs0v77byN74A0vw@mail.gmail.com","subject":"[PATCH] wt-status: quote paths identically whether tracked or untracked","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-09-08T01:30:13Z","receivedAt":"2020-09-08T01:30:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The documentation for git status --short says this:\n\n If a filename contains whitespace or other nonprintable characters,\n that field will be quoted in the manner of a C string literal:\n surrounded by ASCII double quote (34) characters, and with interior\n special characters backslash-escaped.\n\nNote that this differs from our typical quoting rule, which does not\ninclude spaces. If we did not quote spaces, our output would be\nambiguous for renames.\n\nHowever, we failed to do this correctly for untracked files. If we list\nan untracked file that contains spaces, we fail to quote it. Since this\nis both inconsistent and not what was documented, let's fix it by\nquoting untracked files in the same way as tracked files. Users parsing\nthis output already need to handle quoted names for tracked files (or\nuse -z) so this shouldn't be an incompatible change.\n\nNote that the test for this case should be portable because all known\nmodern systems support spaces in file names and our test trash\ndirectories use them already.\n\nReported-by: Patrick Fong \nSigned-off-by: brian m. carlson \n---\n t/t7508-status.sh | 21 +++++++++++++++++++++\n wt-status.c | 8 +++++++-\n 2 files changed, 28 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex e81759319f..ef8d19c151 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -814,6 +814,27 @@ test_expect_success 'status -s without relative paths' '\n \n '\n \n+cat >expect <<\\EOF\n+ M dir1/modified\n+A dir2/added\n+A \"file with spaces\"\n+?? dir1/untracked\n+?? dir2/modified\n+?? dir2/untracked\n+?? \"file with spaces 2\"\n+?? untracked\n+EOF\n+\n+test_expect_success 'status -s without relative paths' '\n+\ttest_when_finished \"git rm --cached \\\"file with spaces\\\"; rm -f file*\" &&\n+\t>\"file with spaces\" &&\n+\t>\"file with spaces 2\" &&\n+\tgit add \"file with spaces\" &&\n+\tgit status -s >output &&\n+\ttest_cmp expect output\n+\n+'\n+\n test_expect_success 'dry-run of partial commit excluding new file in index' '\n \tcat >expect <string, s->prefix, &onebuf);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n-\t\tprintf(\" %s\\n\", one);\n+\t\tputchar(' ');\n+\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n+\t\t\tputchar('\"');\n+\t\t\tstrbuf_addch(&onebuf, '\"');\n+\t\t\tone = onebuf.buf;\n+\t\t}\n+\t\tprintf(\"%s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n }\n"},{"id":"405147","messageId":"xmqq1rjc284j.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"xmqq5z8p12ds.fsf@gitster.c.googlers.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T04:41:00Z","receivedAt":"2020-09-08T04:41:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"brian m. carlson\" writes:\n>\n>> git-status(1) says:\n>>\n>> If a filename contains whitespace or other nonprintable characters,\n>> that field will be quoted in the manner of a C string literal:\n>> surrounded by ASCII double quote (34) characters, and with interior\n>> special characters backslash-escaped.\n>>\n>> Note that that differs from the standard behavior of not handling\n>> spaces, which I expect is due to the need to handle renames\n>> unambiguously.\n>\n> Not really. We use \"rename from\" and \"rename to\" extended header\n> lines in our output to unambiguously handle renamed paths.\n\nAh, I think I misunderstood you. We don't special case SP in what\nyou called \"the standard behaviour\", i.e. \"diff\" output because we\nuse \"rename from/to\", but a short format \"status\" that does not use\nNUL delimited output, we do need to give two paths, separate by\nwhitespaces, and the quoting each path separately does help.\n\n"},{"id":"405166","messageId":"xmqq4ko8yxp9.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"xmqq5z8p12ds.fsf@gitster.c.googlers.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T17:39:46Z","receivedAt":"2020-09-08T17:41:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I agree with your \"the special case handling needs to be taught to\n> the wt_shortstatus_other()\"; a refactored helper function called\n> by both places would help.\n\nI came up with this.\n\n - I very much like the fact that I got rid of the \"directly print\n dq and then feed the remainder of (un)quoted path plus trailing\n dq to the normal printing logic\" from print_cquoted(), even\n though strbuf_insertstr() a single byte to the beginning of the\n buffer feels a bit wasteful.\n\n - I think the short status output for unmerged paths deserve the\n same quoting treatment, so an extra helper function pays off even\n better than our plan to fix \"untracked/ignored\".\n\n - I am undecided if I like that the helper formats and also prints;\n I was hoping I can come up with a pure formatting helper that\n does not do I/O but it seems to be hard to arrange for the\n current callers.\n\nIt seems to pass your tests, but I am not sure how good our test\ncoverage is around this area.\n\nI see some mixed use of stdout and s->fp in the vicinity together\nwith \"fprintf(stdout, ...)\". We may want to clean them up someday,\nby the way.\n\n wt-status.c | 56 ++++++++++++++++++++++----------------------------------\n 1 file changed, 22 insertions(+), 34 deletions(-)\n\ndiff --git c/wt-status.c w/wt-status.c\nindex bb0f9120de..ff43932402 100644\n--- c/wt-status.c\n+++ w/wt-status.c\n@@ -1829,6 +1829,21 @@ static void wt_longstatus_print(struct wt_status *s)\n \t\twt_longstatus_print_stash_summary(s);\n }\n \n+static void print_cquoted(const char *fmt, const char *path, const char *prefix)\n+{\n+\tstruct strbuf onebuf = STRBUF_INIT;\n+\tconst char *one;\n+\n+\tone = quote_path(path, prefix, &onebuf);\n+\tif (*one != '\"' && strchr(one, ' ')) {\n+\t\tstrbuf_insertstr(&onebuf, 0, \"\\\"\");\n+\t\tstrbuf_addch(&onebuf, '\"');\n+\t\tone = onebuf.buf;\n+\t}\n+\tprintf(fmt, one);\n+\tstrbuf_release(&onebuf);\n+}\n+\n static void wt_shortstatus_unmerged(struct string_list_item *it,\n \t\t\t struct wt_status *s)\n {\n@@ -1845,15 +1860,10 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n \tcase 7: how = \"UU\"; break; /* both modified */\n \t}\n \tcolor_fprintf(s->fp, color(WT_STATUS_UNMERGED, s), \"%s\", how);\n-\tif (s->null_termination) {\n+\tif (s->null_termination)\n \t\tfprintf(stdout, \" %s%c\", it->string, 0);\n-\t} else {\n-\t\tstruct strbuf onebuf = STRBUF_INIT;\n-\t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n-\t\tprintf(\" %s\\n\", one);\n-\t\tstrbuf_release(&onebuf);\n-\t}\n+\telse\n+\t\tprint_cquoted(\" %s\\n\", it->string, s->prefix);\n }\n \n static void wt_shortstatus_status(struct string_list_item *it,\n@@ -1875,27 +1885,9 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\tif (d->rename_source)\n \t\t\tfprintf(stdout, \"%s%c\", d->rename_source, 0);\n \t} else {\n-\t\tstruct strbuf onebuf = STRBUF_INIT;\n-\t\tconst char *one;\n-\n-\t\tif (d->rename_source) {\n-\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf);\n-\t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n-\t\t\t\tputchar('\"');\n-\t\t\t\tstrbuf_addch(&onebuf, '\"');\n-\t\t\t\tone = onebuf.buf;\n-\t\t\t}\n-\t\t\tprintf(\"%s -> \", one);\n-\t\t\tstrbuf_release(&onebuf);\n-\t\t}\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n-\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n-\t\t\tputchar('\"');\n-\t\t\tstrbuf_addch(&onebuf, '\"');\n-\t\t\tone = onebuf.buf;\n-\t\t}\n-\t\tprintf(\"%s\\n\", one);\n-\t\tstrbuf_release(&onebuf);\n+\t\tif (d->rename_source)\n+\t\t\tprint_cquoted(\"%s -> \", d->rename_source, s->prefix);\n+\t\tprint_cquoted(\"%s\\n\", it->string, s->prefix);\n \t}\n }\n \n@@ -1905,12 +1897,8 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \tif (s->null_termination) {\n \t\tfprintf(stdout, \"%s %s%c\", sign, it->string, 0);\n \t} else {\n-\t\tstruct strbuf onebuf = STRBUF_INIT;\n-\t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n-\t\tprintf(\" %s\\n\", one);\n-\t\tstrbuf_release(&onebuf);\n+\t\tprint_cquoted(\" %s\\n\", it->string, s->prefix);\n \t}\n }\n \n"},{"id":"405172","messageId":"CAN0heSqJDbMB5=p+OBd7ucph3-XCb+WSi4HR41pVa_0AMsGPWA@mail.gmail.com","threadId":"54206","inReplyTo":"xmqq4ko8yxp9.fsf@gitster.c.googlers.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2020-09-08T19:01:26Z","receivedAt":"2020-09-08T19:01:53Z","isPatch":false,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Tue, 8 Sep 2020 at 19:44, Junio C Hamano wrote:\n> I see some mixed use of stdout and s->fp in the vicinity together\n> with \"fprintf(stdout, ...)\". We may want to clean them up someday,\n> by the way.\n\nI noticed the same thing. I've come up with a few patches to address\nthat and some other minor things I saw on the way. I'll need to test\nsome more before posting, so this is mostly a heads-up for now in case\nothers are thinking about jumping on this.\n\nWill try to post it tomorrow. There's nothing there that needs to block\nany fixes to the escaping, obviously.\n\nMartin\n"},{"id":"405192","messageId":"20200908205224.4126551-1-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908013013.1099937-1-sandals@crustytoothpaste.net","subject":"[PATCH 0/6] quote_path() clean-ups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:18Z","receivedAt":"2020-09-08T20:52:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So, this is an alternative approach to tackle the same issue\n tried to address. It\nended up to be more involved than I would have liked, but it was\nprimarily because it needed some function signature changes.\n \nThe overall structure of the series is:\n\n (1) rename quote_path_relative() to quote_path().\n\n (2) allow quote_path() to take a flags parameter and update all the\n callers to pass 0 (all bits false)\n\n (3) move the \"always quote a path with SP in it, even if there is\n no byte that needs quoting\" logic out of wt-status.c that is\n used to show \"git status --short\" for tracked paths to\n quote_path() and give it the QUOTE_PATH_QUOTE_SP bit in the\n flags parameter.\n\n (4) using QUOTE_PATH_QUOTE_SP bit, apply the \"always quote a path\n with SP\" to \"git status --short\" output for untracked, ignored\n and unmerged paths.\n\nPatches (5) and (6) are not strictly necessary. They are what I\nfound while reading the existing code and thought it might, or might\nnot, be worth cleaning up and couldn't decide ;-)\n\nJunio C Hamano (6):\n quote_path: rename quote_path_relative() to quote_path()\n quote_path: give flags parameter to quote_path()\n quote_path: optionally allow quoting a path with SP in it\n wt-status: consistently quote paths in \"status --short\" output\n quote: rename misnamed sq_lookup[] to cq_lookup[]\n quote: turn 'nodq' parameter into a set of flags\n\n builtin/clean.c | 22 +++++++++++-----------\n builtin/grep.c | 2 +-\n diff.c | 8 ++++----\n quote.c | 39 ++++++++++++++++++++++++---------------\n quote.h | 11 +++++++----\n t/t7508-status.sh | 21 +++++++++++++++++++++\n wt-status.c | 37 +++++++++++++------------------------\n 7 files changed, 81 insertions(+), 59 deletions(-)\n\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405193","messageId":"20200908205224.4126551-3-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH 2/6] quote_path: give flags parameter to quote_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:20Z","receivedAt":"2020-09-08T20:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The quote_path() function computes a path (relative to its base\ndirectory) and c-quotes the result if necessary. Teach it to take a\nflags parameter to allow its behaviour to be enriched later.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 22 +++++++++++-----------\n builtin/grep.c | 2 +-\n quote.c | 4 ++--\n quote.h | 2 +-\n wt-status.c | 24 ++++++++++++------------\n 5 files changed, 27 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex dee44fff6e..687ab473c2 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -162,7 +162,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \tif ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n \t is_nonbare_repository_dir(path)) {\n \t\tif (!quiet) {\n-\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\tprintf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n \t\t\t\t\tquoted.buf);\n \t\t}\n@@ -177,7 +177,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\tres = dry_run ? 0 : rmdir(path->buf);\n \t\tif (res) {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -202,7 +202,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))\n \t\t\t\tret = 1;\n \t\t\tif (gone) {\n-\t\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else\n \t\t\t\t*dir_gone = 0;\n@@ -210,11 +210,11 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(path->buf);\n \t\t\tif (!res) {\n-\t\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t\t*dir_gone = 0;\n@@ -238,7 +238,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\t*dir_gone = 1;\n \t\telse {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -266,7 +266,7 @@ static void pretty_print_dels(void)\n \tstruct column_options copts;\n \n \tfor_each_string_list_item(item, &del_list) {\n-\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\tstring_list_append(&list, qname);\n \t}\n \n@@ -753,7 +753,7 @@ static int ask_each_cmd(void)\n \tfor_each_string_list_item(item, &del_list) {\n \t\t/* Ctrl-D should stop removing files */\n \t\tif (!eof) {\n-\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t/* TRANSLATORS: Make sure to keep [y/N] as is */\n \t\t\tprintf(_(\"Remove %s [y/N]? \"), qname);\n \t\t\tif (git_read_line_interactively(&confirm) == EOF) {\n@@ -1047,19 +1047,19 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\tif (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))\n \t\t\t\terrors++;\n \t\t\tif (gone && !quiet) {\n-\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(abs_path.buf);\n \t\t\tif (res) {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 9a91ad643a..c8037388c6 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -319,7 +319,7 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,\n \t}\n \n \tif (opt->relative && opt->prefix_length)\n-\t\tquote_path(filename + tree_name_len, opt->prefix, out);\n+\t\tquote_path(filename + tree_name_len, opt->prefix, out, 0);\n \telse\n \t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n \ndiff --git a/quote.c b/quote.c\nindex 7bb519c1a7..03ca85afb0 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -352,12 +352,12 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n }\n \n /* quote path as relative to the given prefix */\n-char *quote_path(const char *in, const char *prefix, struct strbuf *out)\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\n \tstrbuf_reset(out);\n-\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n+\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n \tstrbuf_release(&sb);\n \n \treturn out->buf;\ndiff --git a/quote.h b/quote.h\nindex 837cb42a71..db7140b3c7 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -72,7 +72,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator);\n \n /* quote path as relative to the given prefix */\n-char *quote_path(const char *in, const char *prefix, struct strbuf *out);\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned);\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex 6b87592856..d6ca7bd52c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -336,7 +336,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \t\tmemset(padding, ' ', label_width);\n \t}\n \n-\tone = quote_path(it->string, s->prefix, &onebuf);\n+\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \n \thow = wt_status_unmerged_status_string(d->stagemask);\n@@ -402,8 +402,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \tif (d->rename_status == status)\n \t\tone_name = d->rename_source;\n \n-\tone = quote_path(one_name, s->prefix, &onebuf);\n-\ttwo = quote_path(two_name, s->prefix, &twobuf);\n+\tone = quote_path(one_name, s->prefix, &onebuf, 0);\n+\ttwo = quote_path(two_name, s->prefix, &twobuf, 0);\n \n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \twhat = wt_status_diff_status_string(status);\n@@ -964,7 +964,7 @@ static void wt_longstatus_print_other(struct wt_status *s,\n \t\tstruct string_list_item *it;\n \t\tconst char *path;\n \t\tit = &(l->items[i]);\n-\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tpath = quote_path(it->string, s->prefix, &buf, 0);\n \t\tif (column_active(s->colopts)) {\n \t\t\tstring_list_append(&output, path);\n \t\t\tcontinue;\n@@ -1848,7 +1848,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n@@ -1877,7 +1877,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\tconst char *one;\n \n \t\tif (d->rename_source) {\n-\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf);\n+\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf, 0);\n \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\t\tputchar('\"');\n \t\t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1886,7 +1886,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\tprintf(\"%s -> \", one);\n \t\t\tstrbuf_release(&onebuf);\n \t\t}\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\tputchar('\"');\n \t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1905,7 +1905,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n@@ -2222,9 +2222,9 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t */\n \t\tsep_char = '\\t';\n \t\teol_char = '\\n';\n-\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tpath = quote_path(it->string, s->prefix, &buf, 0);\n \t\tif (d->rename_source)\n-\t\t\tpath_from = quote_path(d->rename_source, s->prefix, &buf_from);\n+\t\t\tpath_from = quote_path(d->rename_source, s->prefix, &buf_from, 0);\n \t}\n \n \tif (path_from)\n@@ -2310,7 +2310,7 @@ static void wt_porcelain_v2_print_unmerged_entry(\n \tif (s->null_termination)\n \t\tpath_index = it->string;\n \telse\n-\t\tpath_index = quote_path(it->string, s->prefix, &buf_index);\n+\t\tpath_index = quote_path(it->string, s->prefix, &buf_index, 0);\n \n \tfprintf(s->fp, \"%c %s %s %06o %06o %06o %06o %s %s %s %s%c\",\n \t\t\tunmerged_prefix, key, submodule_token,\n@@ -2343,7 +2343,7 @@ static void wt_porcelain_v2_print_other(\n \t\tpath = it->string;\n \t\teol_char = '\\0';\n \t} else {\n-\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tpath = quote_path(it->string, s->prefix, &buf, 0);\n \t\teol_char = '\\n';\n \t}\n \n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405194","messageId":"20200908205224.4126551-4-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH 3/6] quote_path: optionally allow quoting a path with SP in it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:21Z","receivedAt":"2020-09-08T20:52:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Some code in wt-status.c special case a path with SP in it, which\nusually does not have to be c-quoted, and ensure that such a path\ndoes get quoted. Move the logic to quote_path() and give it a bit\nin the flags word, QUOTE_PATH_QUOTE_SP.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n quote.c | 9 ++++++++-\n quote.h | 1 +\n wt-status.c | 15 +++------------\n 3 files changed, 12 insertions(+), 13 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex 03ca85afb0..aa9a37b1b1 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -357,9 +357,16 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\n \tstrbuf_reset(out);\n-\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n+\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n \tstrbuf_release(&sb);\n \n+\tif ((flags & QUOTE_PATH_QUOTE_SP) &&\n+\t (out->buf[0] != '\"' && strchr(out->buf, ' '))) {\n+\t\t/* Ensure the whole thing is quoted if the path has SP in it */\n+\t\tstrbuf_insertstr(out, 0, \"\\\"\");\n+\t\tstrbuf_addch(out, '\"');\n+\t}\n+\n \treturn out->buf;\n }\n \ndiff --git a/quote.h b/quote.h\nindex db7140b3c7..823f56cb94 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -73,6 +73,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \n /* quote path as relative to the given prefix */\n char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned);\n+#define QUOTE_PATH_QUOTE_SP 01\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex d6ca7bd52c..adbf6958bd 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1877,21 +1877,12 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\tconst char *one;\n \n \t\tif (d->rename_source) {\n-\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf, 0);\n-\t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n-\t\t\t\tputchar('\"');\n-\t\t\t\tstrbuf_addch(&onebuf, '\"');\n-\t\t\t\tone = onebuf.buf;\n-\t\t\t}\n+\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf,\n+\t\t\t\t\t QUOTE_PATH_QUOTE_SP);\n \t\t\tprintf(\"%s -> \", one);\n \t\t\tstrbuf_release(&onebuf);\n \t\t}\n-\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n-\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n-\t\t\tputchar('\"');\n-\t\t\tstrbuf_addch(&onebuf, '\"');\n-\t\t\tone = onebuf.buf;\n-\t\t}\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);\n \t\tprintf(\"%s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405195","messageId":"20200908205224.4126551-5-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH 4/6] wt-status: consistently quote paths in \"status --short\" output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:22Z","receivedAt":"2020-09-08T20:52:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tracked paths with SP in them were c-quoted in \"git status --short\"\noutput, but untracked, ignored, and unmerged paths weren't. Treat\nthem the same way for consistency.\n\nThe test was stolen from a patch to fix output for the 'untracked'\npaths by brian m. carlson.\n\nSigned-off-by: Junio C Hamano \n---\n t/t7508-status.sh | 21 +++++++++++++++++++++\n wt-status.c | 4 ++--\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex e81759319f..ef8d19c151 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -814,6 +814,27 @@ test_expect_success 'status -s without relative paths' '\n \n '\n \n+cat >expect <<\\EOF\n+ M dir1/modified\n+A dir2/added\n+A \"file with spaces\"\n+?? dir1/untracked\n+?? dir2/modified\n+?? dir2/untracked\n+?? \"file with spaces 2\"\n+?? untracked\n+EOF\n+\n+test_expect_success 'status -s without relative paths' '\n+\ttest_when_finished \"git rm --cached \\\"file with spaces\\\"; rm -f file*\" &&\n+\t>\"file with spaces\" &&\n+\t>\"file with spaces 2\" &&\n+\tgit add \"file with spaces\" &&\n+\tgit status -s >output &&\n+\ttest_cmp expect output\n+\n+'\n+\n test_expect_success 'dry-run of partial commit excluding new file in index' '\n \tcat >expect <string, s->prefix, &onebuf, 0);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n@@ -1896,7 +1896,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405196","messageId":"20200908205224.4126551-2-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH 1/6] quote_path: rename quote_path_relative() to quote_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:19Z","receivedAt":"2020-09-08T20:52:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is no quote_path_absolute() or anything that causes confusion,\nand one of the two large consumers already rename the long name\nlocally with a preprocessor macro.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 22 +++++++++++-----------\n builtin/grep.c | 2 +-\n quote.c | 3 +--\n quote.h | 3 +--\n wt-status.c | 2 --\n 5 files changed, 14 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex e53ea52d89..dee44fff6e 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -162,7 +162,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \tif ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n \t is_nonbare_repository_dir(path)) {\n \t\tif (!quiet) {\n-\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\tprintf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n \t\t\t\t\tquoted.buf);\n \t\t}\n@@ -177,7 +177,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\tres = dry_run ? 0 : rmdir(path->buf);\n \t\tif (res) {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -202,7 +202,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))\n \t\t\t\tret = 1;\n \t\t\tif (gone) {\n-\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else\n \t\t\t\t*dir_gone = 0;\n@@ -210,11 +210,11 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(path->buf);\n \t\t\tif (!res) {\n-\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t\t*dir_gone = 0;\n@@ -238,7 +238,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\t*dir_gone = 1;\n \t\telse {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -266,7 +266,7 @@ static void pretty_print_dels(void)\n \tstruct column_options copts;\n \n \tfor_each_string_list_item(item, &del_list) {\n-\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\tqname = quote_path(item->string, NULL, &buf);\n \t\tstring_list_append(&list, qname);\n \t}\n \n@@ -753,7 +753,7 @@ static int ask_each_cmd(void)\n \tfor_each_string_list_item(item, &del_list) {\n \t\t/* Ctrl-D should stop removing files */\n \t\tif (!eof) {\n-\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t/* TRANSLATORS: Make sure to keep [y/N] as is */\n \t\t\tprintf(_(\"Remove %s [y/N]? \"), qname);\n \t\t\tif (git_read_line_interactively(&confirm) == EOF) {\n@@ -1047,19 +1047,19 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\tif (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))\n \t\t\t\terrors++;\n \t\t\tif (gone && !quiet) {\n-\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(abs_path.buf);\n \t\t\tif (res) {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex f58979bc3f..9a91ad643a 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -319,7 +319,7 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,\n \t}\n \n \tif (opt->relative && opt->prefix_length)\n-\t\tquote_path_relative(filename + tree_name_len, opt->prefix, out);\n+\t\tquote_path(filename + tree_name_len, opt->prefix, out);\n \telse\n \t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n \ndiff --git a/quote.c b/quote.c\nindex ced0245e80..7bb519c1a7 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -352,8 +352,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n }\n \n /* quote path as relative to the given prefix */\n-char *quote_path_relative(const char *in, const char *prefix,\n-\t\t\t struct strbuf *out)\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\ndiff --git a/quote.h b/quote.h\nindex fa09309cf6..837cb42a71 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -72,8 +72,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator);\n \n /* quote path as relative to the given prefix */\n-char *quote_path_relative(const char *in, const char *prefix,\n-\t\t\t struct strbuf *out);\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out);\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex bb0f9120de..6b87592856 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -259,8 +259,6 @@ static void wt_longstatus_print_trailer(struct wt_status *s)\n \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n }\n \n-#define quote_path quote_path_relative\n-\n static const char *wt_status_unmerged_status_string(int stagemask)\n {\n \tswitch (stagemask) {\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405197","messageId":"20200908205224.4126551-7-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH 6/6] quote: turn 'nodq' parameter into a set of flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:24Z","receivedAt":"2020-09-08T20:52:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"quote_c_style() and its friend quote_two_c_style() both take an\noptional \"please omit the double quotes around the quoted body\"\nparameter. Turn it into a flag word, assign one bit out of it,\nand call it CQUOTE_NODQ bit.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 8 ++++----\n quote.c | 15 +++++++++------\n quote.h | 7 +++++--\n 3 files changed, 18 insertions(+), 12 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 0299a73079..e7d6e60b23 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -482,14 +482,14 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)\n \n static char *quote_two(const char *one, const char *two)\n {\n-\tint need_one = quote_c_style(one, NULL, NULL, 1);\n-\tint need_two = quote_c_style(two, NULL, NULL, 1);\n+\tint need_one = quote_c_style(one, NULL, NULL, CQUOTE_NODQ);\n+\tint need_two = quote_c_style(two, NULL, NULL, CQUOTE_NODQ);\n \tstruct strbuf res = STRBUF_INIT;\n \n \tif (need_one + need_two) {\n \t\tstrbuf_addch(&res, '\"');\n-\t\tquote_c_style(one, &res, NULL, 1);\n-\t\tquote_c_style(two, &res, NULL, 1);\n+\t\tquote_c_style(one, &res, NULL, CQUOTE_NODQ);\n+\t\tquote_c_style(two, &res, NULL, CQUOTE_NODQ);\n \t\tstrbuf_addch(&res, '\"');\n \t} else {\n \t\tstrbuf_addstr(&res, one);\ndiff --git a/quote.c b/quote.c\nindex 016ebf8873..b2f38d0e57 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -256,7 +256,7 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen)\n * Return value is the same as in (1).\n */\n static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n-\t\t\t\t struct strbuf *sb, FILE *fp, int no_dq)\n+\t\t\t\t struct strbuf *sb, FILE *fp, unsigned flags)\n {\n #undef EMIT\n #define EMIT(c) \\\n@@ -272,6 +272,7 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \t\tcount += (l); \\\n \t} while (0)\n \n+\tint no_dq = !!(flags & CQUOTE_NODQ);\n \tsize_t len, count = 0;\n \tconst char *p = name;\n \n@@ -309,19 +310,21 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \treturn count;\n }\n \n-size_t quote_c_style(const char *name, struct strbuf *sb, FILE *fp, int nodq)\n+size_t quote_c_style(const char *name, struct strbuf *sb, FILE *fp, unsigned flags)\n {\n-\treturn quote_c_style_counted(name, -1, sb, fp, nodq);\n+\treturn quote_c_style_counted(name, -1, sb, fp, flags);\n }\n \n-void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path, int nodq)\n+void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path,\n+\t\t unsigned flags)\n {\n+\tint nodq = !!(flags & CQUOTE_NODQ);\n \tif (quote_c_style(prefix, NULL, NULL, 0) ||\n \t quote_c_style(path, NULL, NULL, 0)) {\n \t\tif (!nodq)\n \t\t\tstrbuf_addch(sb, '\"');\n-\t\tquote_c_style(prefix, sb, NULL, 1);\n-\t\tquote_c_style(path, sb, NULL, 1);\n+\t\tquote_c_style(prefix, sb, NULL, CQUOTE_NODQ);\n+\t\tquote_c_style(path, sb, NULL, CQUOTE_NODQ);\n \t\tif (!nodq)\n \t\t\tstrbuf_addch(sb, '\"');\n \t} else {\ndiff --git a/quote.h b/quote.h\nindex 823f56cb94..0e49c61144 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -64,8 +64,11 @@ struct strvec;\n int sq_dequote_to_strvec(char *arg, struct strvec *);\n \n int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n-size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);\n-void quote_two_c_style(struct strbuf *, const char *, const char *, int);\n+\n+/* Bits in the flags parameter to quote_c_style() */\n+#define CQUOTE_NODQ 01\n+size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n+void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n \n void write_name_quoted(const char *name, FILE *, int terminator);\n void write_name_quoted_relative(const char *name, const char *prefix,\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405198","messageId":"20200908205224.4126551-6-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH 5/6] quote: rename misnamed sq_lookup[] to cq_lookup[]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-08T20:52:23Z","receivedAt":"2020-09-08T20:52:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This table is used to see if each byte needs quoting when responding\nto a request to C-quote the string, not quoting with single-quote in\nthe shell style. Similarly, sq_must_quote() is fed each byte from\nthe string being C-quoted.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n quote.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex aa9a37b1b1..016ebf8873 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -210,7 +210,7 @@ int sq_dequote_to_strvec(char *arg, struct strvec *array)\n */\n #define X8(x) x, x, x, x, x, x, x, x\n #define X16(x) X8(x), X8(x)\n-static signed char const sq_lookup[256] = {\n+static signed char const cq_lookup[256] = {\n \t/* 0 1 2 3 4 5 6 7 */\n \t/* 0x00 */ 1, 1, 1, 1, 1, 1, 1, 'a',\n \t/* 0x08 */ 'b', 't', 'n', 'v', 'f', 'r', 1, 1,\n@@ -223,9 +223,9 @@ static signed char const sq_lookup[256] = {\n \t/* 0x80 */ /* set to 0 */\n };\n \n-static inline int sq_must_quote(char c)\n+static inline int cq_must_quote(char c)\n {\n-\treturn sq_lookup[(unsigned char)c] + quote_path_fully > 0;\n+\treturn cq_lookup[(unsigned char)c] + quote_path_fully > 0;\n }\n \n /* returns the longest prefix not needing a quote up to maxlen if positive.\n@@ -235,9 +235,9 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen)\n {\n \tsize_t len;\n \tif (maxlen < 0) {\n-\t\tfor (len = 0; !sq_must_quote(s[len]); len++);\n+\t\tfor (len = 0; !cq_must_quote(s[len]); len++);\n \t} else {\n-\t\tfor (len = 0; len < maxlen && !sq_must_quote(s[len]); len++);\n+\t\tfor (len = 0; len < maxlen && !cq_must_quote(s[len]); len++);\n \t}\n \treturn len;\n }\n@@ -291,8 +291,8 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \t\tch = (unsigned char)*p++;\n \t\tif (maxlen >= 0)\n \t\t\tmaxlen -= len + 1;\n-\t\tif (sq_lookup[ch] >= ' ') {\n-\t\t\tEMIT(sq_lookup[ch]);\n+\t\tif (cq_lookup[ch] >= ' ') {\n+\t\t\tEMIT(cq_lookup[ch]);\n \t\t} else {\n \t\t\tEMIT(((ch >> 6) & 03) + '0');\n \t\t\tEMIT(((ch >> 3) & 07) + '0');\n-- \n2.28.0-539-g66371d8995\n\n"},{"id":"405201","messageId":"3a72c5f2-35cc-a865-d5f2-02706c48d8ec@web.de","threadId":"54206","inReplyTo":"xmqq4ko8yxp9.fsf@gitster.c.googlers.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-09-08T21:06:50Z","receivedAt":"2020-09-08T21:07:08Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 08.09.20 um 19:39 schrieb Junio C Hamano:\n> Junio C Hamano writes:\n>\n>> I agree with your \"the special case handling needs to be taught to\n>> the wt_shortstatus_other()\"; a refactored helper function called\n>> by both places would help.\n>\n> I came up with this.\n>\n> - I very much like the fact that I got rid of the \"directly print\n> dq and then feed the remainder of (un)quoted path plus trailing\n> dq to the normal printing logic\" from print_cquoted(), even\n> though strbuf_insertstr() a single byte to the beginning of the\n> buffer feels a bit wasteful.\n\nI don't particularly like how this print* function takes a printf\nformat, but doesn't interpret its remaining arguments like printf does.\n\n> - I think the short status output for unmerged paths deserve the\n> same quoting treatment, so an extra helper function pays off even\n> better than our plan to fix \"untracked/ignored\".\n>\n> - I am undecided if I like that the helper formats and also prints;\n> I was hoping I can come up with a pure formatting helper that\n> does not do I/O but it seems to be hard to arrange for the\n> current callers.\n\nHow about something like this then?\n\ndiff --git a/wt-status.c b/wt-status.c\nindex ff43932402..e0711aff04 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1829,19 +1829,19 @@ static void wt_longstatus_print(struct wt_status *s)\n \t\twt_longstatus_print_stash_summary(s);\n }\n\n-static void print_cquoted(const char *fmt, const char *path, const char *prefix)\n+static const char *cquote(const char *in, const char *prefix, struct strbuf *out)\n {\n-\tstruct strbuf onebuf = STRBUF_INIT;\n-\tconst char *one;\n-\n-\tone = quote_path(path, prefix, &onebuf);\n-\tif (*one != '\"' && strchr(one, ' ')) {\n-\t\tstrbuf_insertstr(&onebuf, 0, \"\\\"\");\n-\t\tstrbuf_addch(&onebuf, '\"');\n-\t\tone = onebuf.buf;\n-\t}\n-\tprintf(fmt, one);\n-\tstrbuf_release(&onebuf);\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *rel = relative_path(in, prefix, &sb);\n+\tint need_quotes = *rel != '\"' && strchr(rel, ' ');\n+\tstrbuf_reset(out);\n+\tif (need_quotes)\n+\t\tstrbuf_addch(out, '\"');\n+\tquote_c_style(rel, out, NULL, 0);\n+\tif (need_quotes)\n+\t\tstrbuf_addch(out, '\"');\n+\tstrbuf_release(&sb);\n+\treturn out->buf;\n }\n\n static void wt_shortstatus_unmerged(struct string_list_item *it,\n@@ -1849,6 +1849,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n {\n \tstruct wt_status_change_data *d = it->util;\n \tconst char *how = \"??\";\n+\tstruct strbuf sb = STRBUF_INIT;\n\n \tswitch (d->stagemask) {\n \tcase 1: how = \"DD\"; break; /* both deleted */\n@@ -1863,13 +1864,15 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n \tif (s->null_termination)\n \t\tfprintf(stdout, \" %s%c\", it->string, 0);\n \telse\n-\t\tprint_cquoted(\" %s\\n\", it->string, s->prefix);\n+\t\tprintf(\" %s\\n\", cquote(it->string, s->prefix, &sb));\n+\tstrbuf_release(&sb);\n }\n\n static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\t struct wt_status *s)\n {\n \tstruct wt_status_change_data *d = it->util;\n+\tstruct strbuf sb = STRBUF_INIT;\n\n \tif (d->index_status)\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UPDATED, s), \"%c\", d->index_status);\n@@ -1886,20 +1889,24 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\tfprintf(stdout, \"%s%c\", d->rename_source, 0);\n \t} else {\n \t\tif (d->rename_source)\n-\t\t\tprint_cquoted(\"%s -> \", d->rename_source, s->prefix);\n-\t\tprint_cquoted(\"%s\\n\", it->string, s->prefix);\n+\t\t\tprintf(\"%s -> \", cquote(d->rename_source, s->prefix, &sb));\n+\t\tprintf(\"%s\\n\", cquote(it->string, s->prefix, &sb));\n \t}\n+\tstrbuf_release(&sb);\n }\n\n static void wt_shortstatus_other(struct string_list_item *it,\n \t\t\t\t struct wt_status *s, const char *sign)\n {\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n \tif (s->null_termination) {\n \t\tfprintf(stdout, \"%s %s%c\", sign, it->string, 0);\n \t} else {\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n-\t\tprint_cquoted(\" %s\\n\", it->string, s->prefix);\n+\t\tprintf(\" %s\\n\", cquote(it->string, s->prefix, &sb));\n \t}\n+\tstrbuf_release(&sb);\n }\n\n static void wt_shortstatus_print_tracking(struct wt_status *s)\n\n"},{"id":"405209","messageId":"CAPx1GvddM_B9Lehg8pX2yEXgiHL5vKYiabiCsjKhKHDnhQTx4g@mail.gmail.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"Re: [PATCH 0/6] quote_path() clean-ups","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2020-09-08T22:56:56Z","receivedAt":"2020-09-08T22:57:10Z","isPatch":true,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Tue, Sep 8, 2020 at 1:56 PM Junio C Hamano wrote:\n>\n> So, this is an alternative approach to tackle the same issue\n> tried to address. It\n> ended up to be more involved than I would have liked, but it was\n> primarily because it needed some function signature changes.\n\n\nNot that it necessarily means much, but I gave the whole series\na quick eyeball scan and it looks good. I like the idea of using one\nconsistent \"quote the path\" routine.\n\nChris\n"},{"id":"405290","messageId":"xmqqa6xytwtn.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"3a72c5f2-35cc-a865-d5f2-02706c48d8ec@web.de","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-09T22:22:12Z","receivedAt":"2020-09-09T22:22:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> I don't particularly like how this print* function takes a printf\n> format, but doesn't interpret its remaining arguments like printf does.\n\nYes. I think the more elaborate one that pushes the logic down to\nquote_path() I posted earlier is much cleaner in this regard.\n\n>> - I am undecided if I like that the helper formats and also prints;\n>> I was hoping I can come up with a pure formatting helper that\n>> does not do I/O but it seems to be hard to arrange for the\n>> current callers.\n>\n> How about something like this then?\n\nThanks. I kind of like the fact that it is isolated compared to the\n\"push the logic down to quote_path()\" approach.\n\nBut it also is a double edged sword. It makes it harder for other\ncodepaths to quote a path with SP in it by mistake, but if it turns\nout to be a good longer term thing to quote a path with SP in it,\nit makes it harder to do so.\n\n> diff --git a/wt-status.c b/wt-status.c\n> index ff43932402..e0711aff04 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1829,19 +1829,19 @@ static void wt_longstatus_print(struct wt_status *s)\n> \t\twt_longstatus_print_stash_summary(s);\n> }\n>\n> -static void print_cquoted(const char *fmt, const char *path, const char *prefix)\n> +static const char *cquote(const char *in, const char *prefix, struct strbuf *out)\n> {\n> -\tstruct strbuf onebuf = STRBUF_INIT;\n> -\tconst char *one;\n> -\n> -\tone = quote_path(path, prefix, &onebuf);\n> -\tif (*one != '\"' && strchr(one, ' ')) {\n> -\t\tstrbuf_insertstr(&onebuf, 0, \"\\\"\");\n> -\t\tstrbuf_addch(&onebuf, '\"');\n> -\t\tone = onebuf.buf;\n> -\t}\n> -\tprintf(fmt, one);\n> -\tstrbuf_release(&onebuf);\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tconst char *rel = relative_path(in, prefix, &sb);\n> +\tint need_quotes = *rel != '\"' && strchr(rel, ' ');\n\nrelative_path() does not quote, so \"begins with a dq\" is not a good\ntest to see \"if we were to pass this string to quote_c_style(), would\nwe get it back quoted already so we won't have to surround the\nresult with an extra pair of dq ourselves?\".\n\n> +\tstrbuf_reset(out);\n> +\tif (need_quotes)\n> +\t\tstrbuf_addch(out, '\"');\n> +\tquote_c_style(rel, out, NULL, 0);\n> +\tif (need_quotes)\n> +\t\tstrbuf_addch(out, '\"');\n> +\tstrbuf_release(&sb);\n> +\treturn out->buf;\n> }\n"},{"id":"405325","messageId":"20200910123509.GB57925@coredump.intra.peff.net","threadId":"54206","inReplyTo":"20200908205224.4126551-4-gitster@pobox.com","subject":"Re: [PATCH 3/6] quote_path: optionally allow quoting a path with SP in it","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T12:35:09Z","receivedAt":"2020-09-10T12:40:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 08, 2020 at 01:52:21PM -0700, Junio C Hamano wrote:\n\n> Some code in wt-status.c special case a path with SP in it, which\n> usually does not have to be c-quoted, and ensure that such a path\n> does get quoted. Move the logic to quote_path() and give it a bit\n> in the flags word, QUOTE_PATH_QUOTE_SP.\n> \n> No behaviour change intended.\n\nSounds like a good direction.\n\n> @@ -357,9 +357,16 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n> \tstruct strbuf sb = STRBUF_INIT;\n> \tconst char *rel = relative_path(in, prefix, &sb);\n> \tstrbuf_reset(out);\n> -\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n> \tstrbuf_release(&sb);\n> \n> +\tif ((flags & QUOTE_PATH_QUOTE_SP) &&\n> +\t (out->buf[0] != '\"' && strchr(out->buf, ' '))) {\n> +\t\t/* Ensure the whole thing is quoted if the path has SP in it */\n> +\t\tstrbuf_insertstr(out, 0, \"\\\"\");\n> +\t\tstrbuf_addch(out, '\"');\n> +\t}\n\nThis might be premature optimization, but using insertstr() means we\nhave to recopy the entire string. As a general rule these kind of\n\"splice into an array\" operations tickle my accidentally-quadratic\nspider sense. But I don't think that's the case here (because we'd do at\nmost one insertstr() per string; the real sin would be inserting\ncharacters like that throughout the string).\n\nSo it may not be worth addressing. I do think it would be conceptually\nsimpler if we could just tell quote_c_style_counted() to look for space\ncharacters, but it looks like doing that ends up pretty invasive.\n\n-Peff\n"},{"id":"405326","messageId":"20200910123944.GD57925@coredump.intra.peff.net","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"Re: [PATCH 0/6] quote_path() clean-ups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T12:39:44Z","receivedAt":"2020-09-10T12:42:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 08, 2020 at 01:52:18PM -0700, Junio C Hamano wrote:\n\n> So, this is an alternative approach to tackle the same issue\n> tried to address. It\n> ended up to be more involved than I would have liked, but it was\n> primarily because it needed some function signature changes.\n\nThis looks like an improvement to me. I left a few minor comments that\nmight be worth addressing, and a few whose resolution is probably \"well,\nwe _could_ do that but it's not worth spending more time on this\".\n\n-Peff\n"},{"id":"405328","messageId":"20200910170159.1278781-2-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 1/7] quote_path: rename quote_path_relative() to quote_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:53Z","receivedAt":"2020-09-10T17:03:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is no quote_path_absolute() or anything that causes confusion,\nand one of the two large consumers already rename the long name\nlocally with a preprocessor macro.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 22 +++++++++++-----------\n builtin/grep.c | 2 +-\n quote.c | 3 +--\n quote.h | 3 +--\n wt-status.c | 2 --\n 5 files changed, 14 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex e53ea52d89..dee44fff6e 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -162,7 +162,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \tif ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n \t is_nonbare_repository_dir(path)) {\n \t\tif (!quiet) {\n-\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\tprintf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n \t\t\t\t\tquoted.buf);\n \t\t}\n@@ -177,7 +177,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\tres = dry_run ? 0 : rmdir(path->buf);\n \t\tif (res) {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -202,7 +202,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))\n \t\t\t\tret = 1;\n \t\t\tif (gone) {\n-\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else\n \t\t\t\t*dir_gone = 0;\n@@ -210,11 +210,11 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(path->buf);\n \t\t\tif (!res) {\n-\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t\t*dir_gone = 0;\n@@ -238,7 +238,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\t*dir_gone = 1;\n \t\telse {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path_relative(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -266,7 +266,7 @@ static void pretty_print_dels(void)\n \tstruct column_options copts;\n \n \tfor_each_string_list_item(item, &del_list) {\n-\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\tqname = quote_path(item->string, NULL, &buf);\n \t\tstring_list_append(&list, qname);\n \t}\n \n@@ -753,7 +753,7 @@ static int ask_each_cmd(void)\n \tfor_each_string_list_item(item, &del_list) {\n \t\t/* Ctrl-D should stop removing files */\n \t\tif (!eof) {\n-\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t/* TRANSLATORS: Make sure to keep [y/N] as is */\n \t\t\tprintf(_(\"Remove %s [y/N]? \"), qname);\n \t\t\tif (git_read_line_interactively(&confirm) == EOF) {\n@@ -1047,19 +1047,19 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\tif (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))\n \t\t\t\terrors++;\n \t\t\tif (gone && !quiet) {\n-\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(abs_path.buf);\n \t\t\tif (res) {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex f58979bc3f..9a91ad643a 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -319,7 +319,7 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,\n \t}\n \n \tif (opt->relative && opt->prefix_length)\n-\t\tquote_path_relative(filename + tree_name_len, opt->prefix, out);\n+\t\tquote_path(filename + tree_name_len, opt->prefix, out);\n \telse\n \t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n \ndiff --git a/quote.c b/quote.c\nindex ced0245e80..7bb519c1a7 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -352,8 +352,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n }\n \n /* quote path as relative to the given prefix */\n-char *quote_path_relative(const char *in, const char *prefix,\n-\t\t\t struct strbuf *out)\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\ndiff --git a/quote.h b/quote.h\nindex fa09309cf6..837cb42a71 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -72,8 +72,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator);\n \n /* quote path as relative to the given prefix */\n-char *quote_path_relative(const char *in, const char *prefix,\n-\t\t\t struct strbuf *out);\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out);\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex bb0f9120de..6b87592856 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -259,8 +259,6 @@ static void wt_longstatus_print_trailer(struct wt_status *s)\n \tstatus_printf_ln(s, color(WT_STATUS_HEADER, s), \"%s\", \"\");\n }\n \n-#define quote_path quote_path_relative\n-\n static const char *wt_status_unmerged_status_string(int stagemask)\n {\n \tswitch (stagemask) {\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405329","messageId":"20200910170159.1278781-5-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 4/7] quote_path: code clarification","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:56Z","receivedAt":"2020-09-10T17:03:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The implementation we moved from wt-status to enclose a pathname\nthat has a SP in it inside a dq-pair is a bit convoluted. It lets\nquote_c_style_counted() do its escaping and then\n\n (1) if the input string got escaped, which is checked by seeing if\n the result begins with a double-quote, declare that we are\n done. If there wasn't any SP in the input, that is OK, and if\n there was, the result is quoted already so it is OK, too.\n\n (2) if the input string did not get escaped, and the result has SP\n in it, enclose the whole thing in a dq-pair ourselves.\n\nInstead we can scan the path upfront to see if the input has SP in\nit. If so, we tell quote_c_style_counted() not to enclose its\noutput in a dq-pair, and we add a dq-pair ourselves. Whether the\ninput had bytes that quote_c_style_counted() uses backslash quoting,\nthis would give us a desired quoted string. If the input does not\nhave SP in it, we just let quote_c_style_counted() do its thing as\nusual, which would enclose the result in a dq-pair only when needed.\n\nSigned-off-by: Junio C Hamano \n---\n quote.c | 19 ++++++++++++-------\n 1 file changed, 12 insertions(+), 7 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex aa9a37b1b1..b8107cd403 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -356,16 +356,21 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\n+\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n+\n \tstrbuf_reset(out);\n-\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n-\tstrbuf_release(&sb);\n \n-\tif ((flags & QUOTE_PATH_QUOTE_SP) &&\n-\t (out->buf[0] != '\"' && strchr(out->buf, ' '))) {\n-\t\t/* Ensure the whole thing is quoted if the path has SP in it */\n-\t\tstrbuf_insertstr(out, 0, \"\\\"\");\n+\t/*\n+\t * If the caller wants us to enclose the output in a dq-pair\n+\t * whether quote_c_style_counted() needs to, we do it ourselves\n+\t * and tell quote_c_style_counted() not to.\n+\t */\n+\tif (force_dq)\n \t\tstrbuf_addch(out, '\"');\n-\t}\n+\tquote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq);\n+\tif (force_dq)\n+\t\tstrbuf_addch(out, '\"');\n+\tstrbuf_release(&sb);\n \n \treturn out->buf;\n }\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405330","messageId":"20200910170159.1278781-1-gitster@pobox.com","threadId":"54206","inReplyTo":"20200908205224.4126551-1-gitster@pobox.com","subject":"[PATCH v2 0/7] quote_path() clean-ups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:52Z","receivedAt":"2020-09-10T17:03:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is an update, after seeing Peff's review.\n\nThe overall structure of the series stays the same.\n\n * The second patch lost an incorrect use of 'flags' parameter down\n to quote_c_style_counted() from quote_path(). In the function\n declaration of quote_path(), the flags parameter is now named.\n\n * The third patch that moves the \"optionally quote path with SP in\n it\" logic to quote_path() is essentialy unchanged.\n\n * But the fourth patch rewrites the implementation to avoid\n shifting the output bytes with insertstr().\n\n * The fifth patch (formerly the fourth) that teachs \"status --short\"\n to consistently quote hasn't changed, except that we test also\n for the ignored paths.\n\n * The last two patches are unchanged. They are optional clean-ups\n that are not required.\n\nWe might want to add a bit more tests for:\n\n - how conflicted \"funny\" paths are shown.\n\n - how doubly funny paths (those that quote_c_style_counted() needs\n to use backslash quoting *and* that contain SP) are shown.\n\nbut I'd say that is outside the scope of this round. Seeing what is\ndone in t3300, the latter test cannot be written portably as far as\nI can tell.\n\n\nJunio C Hamano (7):\n quote_path: rename quote_path_relative() to quote_path()\n quote_path: give flags parameter to quote_path()\n quote_path: optionally allow quoting a path with SP in it\n quote_path: code clarification\n wt-status: consistently quote paths in \"status --short\" output\n quote: rename misnamed sq_lookup[] to cq_lookup[]\n quote: turn 'nodq' parameter into a set of flags\n\n builtin/clean.c | 22 +++++++++++-----------\n builtin/grep.c | 2 +-\n diff.c | 8 ++++----\n quote.c | 47 +++++++++++++++++++++++++++++++----------------\n quote.h | 11 +++++++----\n t/t7508-status.sh | 27 +++++++++++++++++++++++++++\n wt-status.c | 37 +++++++++++++------------------------\n 7 files changed, 94 insertions(+), 60 deletions(-)\n\nRange-diff against v1:\n1: a9306429f4 = 1: a9306429f4 quote_path: rename quote_path_relative() to quote_path()\n2: c2784628a4 ! 2: 179d12d16f quote_path: give flags parameter to quote_path()\n @@ quote.c: void write_name_quoted_relative(const char *name, const char *prefix,\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\n - \tstrbuf_reset(out);\n --\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n -+\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n - \tstrbuf_release(&sb);\n - \n - \treturn out->buf;\n \n ## quote.h ##\n @@ quote.h: void write_name_quoted_relative(const char *name, const char *prefix,\n @@ quote.h: void write_name_quoted_relative(const char *name, const char *prefix,\n \n /* quote path as relative to the given prefix */\n -char *quote_path(const char *in, const char *prefix, struct strbuf *out);\n -+char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned);\n ++char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\n3: 640673148e ! 3: d566c38d2f quote_path: optionally allow quoting a path with SP in it\n @@ Commit message\n \n ## quote.c ##\n @@ quote.c: char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n - \tstruct strbuf sb = STRBUF_INIT;\n - \tconst char *rel = relative_path(in, prefix, &sb);\n - \tstrbuf_reset(out);\n --\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n -+\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n + \tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n \tstrbuf_release(&sb);\n \n +\tif ((flags & QUOTE_PATH_QUOTE_SP) &&\n @@ quote.h\n @@ quote.h: void write_name_quoted_relative(const char *name, const char *prefix,\n \n /* quote path as relative to the given prefix */\n - char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned);\n + char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n +#define QUOTE_PATH_QUOTE_SP 01\n \n /* quoting as a string literal for other languages */\n-: ---------- > 4: f3769b7cf4 quote_path: code clarification\n4: ac9a0c4a33 ! 5: 498bed71e4 wt-status: consistently quote paths in \"status --short\" output\n @@ Commit message\n output, but untracked, ignored, and unmerged paths weren't.\n \n The test was stolen from a patch to fix output for the 'untracked'\n - paths by brian m. carlson.\n + paths by brian m. carlson, with similar tests added for 'ignored'\n + ones.\n \n Signed-off-by: Junio C Hamano \n \n @@ t/t7508-status.sh: test_expect_success 'status -s without relative paths' '\n +\ttest_when_finished \"git rm --cached \\\"file with spaces\\\"; rm -f file*\" &&\n +\t>\"file with spaces\" &&\n +\t>\"file with spaces 2\" &&\n ++\t>\"expect with spaces\" &&\n +\tgit add \"file with spaces\" &&\n ++\n +\tgit status -s >output &&\n -+\ttest_cmp expect output\n ++\ttest_cmp expect output &&\n +\n ++\tgit status -s --ignored >output &&\n ++\tgrep \"^!! \\\"expect with spaces\\\"$\" output &&\n ++\tgrep -v \"^!! \" output >output-wo-ignored &&\n ++\ttest_cmp expect output-wo-ignored\n +'\n +\n test_expect_success 'dry-run of partial commit excluding new file in index' '\n5: 57c6294695 = 6: e74186bbd7 quote: rename misnamed sq_lookup[] to cq_lookup[]\n6: 480152cfe4 ! 7: d87d7dd561 quote: turn 'nodq' parameter into a set of flags\n @@ quote.c: static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \t\tif (!nodq)\n \t\t\tstrbuf_addch(sb, '\"');\n \t} else {\n +@@ quote.c: char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n + \t */\n + \tif (force_dq)\n + \t\tstrbuf_addch(out, '\"');\n +-\tquote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq);\n ++\tquote_c_style_counted(rel, strlen(rel), out, NULL,\n ++\t\t\t force_dq ? CQUOTE_NODQ : 0);\n + \tif (force_dq)\n + \t\tstrbuf_addch(out, '\"');\n + \tstrbuf_release(&sb);\n \n ## quote.h ##\n @@ quote.h: struct strvec;\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405331","messageId":"20200910170159.1278781-8-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 7/7] quote: turn 'nodq' parameter into a set of flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:59Z","receivedAt":"2020-09-10T17:03:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"quote_c_style() and its friend quote_two_c_style() both take an\noptional \"please omit the double quotes around the quoted body\"\nparameter. Turn it into a flag word, assign one bit out of it,\nand call it CQUOTE_NODQ bit.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n diff.c | 8 ++++----\n quote.c | 18 +++++++++++-------\n quote.h | 7 +++++--\n 3 files changed, 20 insertions(+), 13 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 0299a73079..e7d6e60b23 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -482,14 +482,14 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)\n \n static char *quote_two(const char *one, const char *two)\n {\n-\tint need_one = quote_c_style(one, NULL, NULL, 1);\n-\tint need_two = quote_c_style(two, NULL, NULL, 1);\n+\tint need_one = quote_c_style(one, NULL, NULL, CQUOTE_NODQ);\n+\tint need_two = quote_c_style(two, NULL, NULL, CQUOTE_NODQ);\n \tstruct strbuf res = STRBUF_INIT;\n \n \tif (need_one + need_two) {\n \t\tstrbuf_addch(&res, '\"');\n-\t\tquote_c_style(one, &res, NULL, 1);\n-\t\tquote_c_style(two, &res, NULL, 1);\n+\t\tquote_c_style(one, &res, NULL, CQUOTE_NODQ);\n+\t\tquote_c_style(two, &res, NULL, CQUOTE_NODQ);\n \t\tstrbuf_addch(&res, '\"');\n \t} else {\n \t\tstrbuf_addstr(&res, one);\ndiff --git a/quote.c b/quote.c\nindex 9a6e0e7dea..69f4ca45da 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -256,7 +256,7 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen)\n * Return value is the same as in (1).\n */\n static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n-\t\t\t\t struct strbuf *sb, FILE *fp, int no_dq)\n+\t\t\t\t struct strbuf *sb, FILE *fp, unsigned flags)\n {\n #undef EMIT\n #define EMIT(c) \\\n@@ -272,6 +272,7 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \t\tcount += (l); \\\n \t} while (0)\n \n+\tint no_dq = !!(flags & CQUOTE_NODQ);\n \tsize_t len, count = 0;\n \tconst char *p = name;\n \n@@ -309,19 +310,21 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \treturn count;\n }\n \n-size_t quote_c_style(const char *name, struct strbuf *sb, FILE *fp, int nodq)\n+size_t quote_c_style(const char *name, struct strbuf *sb, FILE *fp, unsigned flags)\n {\n-\treturn quote_c_style_counted(name, -1, sb, fp, nodq);\n+\treturn quote_c_style_counted(name, -1, sb, fp, flags);\n }\n \n-void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path, int nodq)\n+void quote_two_c_style(struct strbuf *sb, const char *prefix, const char *path,\n+\t\t unsigned flags)\n {\n+\tint nodq = !!(flags & CQUOTE_NODQ);\n \tif (quote_c_style(prefix, NULL, NULL, 0) ||\n \t quote_c_style(path, NULL, NULL, 0)) {\n \t\tif (!nodq)\n \t\t\tstrbuf_addch(sb, '\"');\n-\t\tquote_c_style(prefix, sb, NULL, 1);\n-\t\tquote_c_style(path, sb, NULL, 1);\n+\t\tquote_c_style(prefix, sb, NULL, CQUOTE_NODQ);\n+\t\tquote_c_style(path, sb, NULL, CQUOTE_NODQ);\n \t\tif (!nodq)\n \t\t\tstrbuf_addch(sb, '\"');\n \t} else {\n@@ -367,7 +370,8 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n \t */\n \tif (force_dq)\n \t\tstrbuf_addch(out, '\"');\n-\tquote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq);\n+\tquote_c_style_counted(rel, strlen(rel), out, NULL,\n+\t\t\t force_dq ? CQUOTE_NODQ : 0);\n \tif (force_dq)\n \t\tstrbuf_addch(out, '\"');\n \tstrbuf_release(&sb);\ndiff --git a/quote.h b/quote.h\nindex 1918d1e00e..4b72a583cf 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -64,8 +64,11 @@ struct strvec;\n int sq_dequote_to_strvec(char *arg, struct strvec *);\n \n int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n-size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);\n-void quote_two_c_style(struct strbuf *, const char *, const char *, int);\n+\n+/* Bits in the flags parameter to quote_c_style() */\n+#define CQUOTE_NODQ 01\n+size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n+void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n \n void write_name_quoted(const char *name, FILE *, int terminator);\n void write_name_quoted_relative(const char *name, const char *prefix,\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405332","messageId":"20200910170159.1278781-6-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 5/7] wt-status: consistently quote paths in \"status --short\" output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:57Z","receivedAt":"2020-09-10T17:03:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tracked paths with SP in them were cquoted in \"git status --short\"\noutput, but untracked, ignored, and unmerged paths weren't.\n\nThe test was stolen from a patch to fix output for the 'untracked'\npaths by brian m. carlson, with similar tests added for 'ignored'\nones.\n\nSigned-off-by: Junio C Hamano \n---\n t/t7508-status.sh | 27 +++++++++++++++++++++++++++\n wt-status.c | 4 ++--\n 2 files changed, 29 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex e81759319f..2e9c6daf1a 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -814,6 +814,33 @@ test_expect_success 'status -s without relative paths' '\n \n '\n \n+cat >expect <<\\EOF\n+ M dir1/modified\n+A dir2/added\n+A \"file with spaces\"\n+?? dir1/untracked\n+?? dir2/modified\n+?? dir2/untracked\n+?? \"file with spaces 2\"\n+?? untracked\n+EOF\n+\n+test_expect_success 'status -s without relative paths' '\n+\ttest_when_finished \"git rm --cached \\\"file with spaces\\\"; rm -f file*\" &&\n+\t>\"file with spaces\" &&\n+\t>\"file with spaces 2\" &&\n+\t>\"expect with spaces\" &&\n+\tgit add \"file with spaces\" &&\n+\n+\tgit status -s >output &&\n+\ttest_cmp expect output &&\n+\n+\tgit status -s --ignored >output &&\n+\tgrep \"^!! \\\"expect with spaces\\\"$\" output &&\n+\tgrep -v \"^!! \" output >output-wo-ignored &&\n+\ttest_cmp expect output-wo-ignored\n+'\n+\n test_expect_success 'dry-run of partial commit excluding new file in index' '\n \tcat >expect <string, s->prefix, &onebuf, 0);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n@@ -1896,7 +1896,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405333","messageId":"20200910170159.1278781-3-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 2/7] quote_path: give flags parameter to quote_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:54Z","receivedAt":"2020-09-10T17:03:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The quote_path() function computes a path (relative to its base\ndirectory) and c-quotes the result if necessary. Teach it to take a\nflags parameter to allow its behaviour to be enriched later.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 22 +++++++++++-----------\n builtin/grep.c | 2 +-\n quote.c | 2 +-\n quote.h | 2 +-\n wt-status.c | 24 ++++++++++++------------\n 5 files changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex dee44fff6e..687ab473c2 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -162,7 +162,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \tif ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n \t is_nonbare_repository_dir(path)) {\n \t\tif (!quiet) {\n-\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\tprintf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n \t\t\t\t\tquoted.buf);\n \t\t}\n@@ -177,7 +177,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\tres = dry_run ? 0 : rmdir(path->buf);\n \t\tif (res) {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -202,7 +202,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))\n \t\t\t\tret = 1;\n \t\t\tif (gone) {\n-\t\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else\n \t\t\t\t*dir_gone = 0;\n@@ -210,11 +210,11 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(path->buf);\n \t\t\tif (!res) {\n-\t\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t\t*dir_gone = 0;\n@@ -238,7 +238,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\t*dir_gone = 1;\n \t\telse {\n \t\t\tint saved_errno = errno;\n-\t\t\tquote_path(path->buf, prefix, "ed);\n+\t\t\tquote_path(path->buf, prefix, "ed, 0);\n \t\t\terrno = saved_errno;\n \t\t\twarning_errno(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n@@ -266,7 +266,7 @@ static void pretty_print_dels(void)\n \tstruct column_options copts;\n \n \tfor_each_string_list_item(item, &del_list) {\n-\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\tstring_list_append(&list, qname);\n \t}\n \n@@ -753,7 +753,7 @@ static int ask_each_cmd(void)\n \tfor_each_string_list_item(item, &del_list) {\n \t\t/* Ctrl-D should stop removing files */\n \t\tif (!eof) {\n-\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t/* TRANSLATORS: Make sure to keep [y/N] as is */\n \t\t\tprintf(_(\"Remove %s [y/N]? \"), qname);\n \t\t\tif (git_read_line_interactively(&confirm) == EOF) {\n@@ -1047,19 +1047,19 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\tif (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))\n \t\t\t\terrors++;\n \t\t\tif (gone && !quiet) {\n-\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(abs_path.buf);\n \t\t\tif (res) {\n \t\t\t\tint saved_errno = errno;\n-\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t\terrno = saved_errno;\n \t\t\t\twarning_errno(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path(item->string, NULL, &buf);\n+\t\t\t\tqname = quote_path(item->string, NULL, &buf, 0);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 9a91ad643a..c8037388c6 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -319,7 +319,7 @@ static void grep_source_name(struct grep_opt *opt, const char *filename,\n \t}\n \n \tif (opt->relative && opt->prefix_length)\n-\t\tquote_path(filename + tree_name_len, opt->prefix, out);\n+\t\tquote_path(filename + tree_name_len, opt->prefix, out, 0);\n \telse\n \t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n \ndiff --git a/quote.c b/quote.c\nindex 7bb519c1a7..a86f9f22a2 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -352,7 +352,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n }\n \n /* quote path as relative to the given prefix */\n-char *quote_path(const char *in, const char *prefix, struct strbuf *out)\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\ndiff --git a/quote.h b/quote.h\nindex 837cb42a71..4687b5daf4 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -72,7 +72,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator);\n \n /* quote path as relative to the given prefix */\n-char *quote_path(const char *in, const char *prefix, struct strbuf *out);\n+char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex 6b87592856..d6ca7bd52c 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -336,7 +336,7 @@ static void wt_longstatus_print_unmerged_data(struct wt_status *s,\n \t\tmemset(padding, ' ', label_width);\n \t}\n \n-\tone = quote_path(it->string, s->prefix, &onebuf);\n+\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \n \thow = wt_status_unmerged_status_string(d->stagemask);\n@@ -402,8 +402,8 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \tif (d->rename_status == status)\n \t\tone_name = d->rename_source;\n \n-\tone = quote_path(one_name, s->prefix, &onebuf);\n-\ttwo = quote_path(two_name, s->prefix, &twobuf);\n+\tone = quote_path(one_name, s->prefix, &onebuf, 0);\n+\ttwo = quote_path(two_name, s->prefix, &twobuf, 0);\n \n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \twhat = wt_status_diff_status_string(status);\n@@ -964,7 +964,7 @@ static void wt_longstatus_print_other(struct wt_status *s,\n \t\tstruct string_list_item *it;\n \t\tconst char *path;\n \t\tit = &(l->items[i]);\n-\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tpath = quote_path(it->string, s->prefix, &buf, 0);\n \t\tif (column_active(s->colopts)) {\n \t\t\tstring_list_append(&output, path);\n \t\t\tcontinue;\n@@ -1848,7 +1848,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n@@ -1877,7 +1877,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\tconst char *one;\n \n \t\tif (d->rename_source) {\n-\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf);\n+\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf, 0);\n \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\t\tputchar('\"');\n \t\t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1886,7 +1886,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\tprintf(\"%s -> \", one);\n \t\t\tstrbuf_release(&onebuf);\n \t\t}\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\tputchar('\"');\n \t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1905,7 +1905,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, s->prefix, &onebuf);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n@@ -2222,9 +2222,9 @@ static void wt_porcelain_v2_print_changed_entry(\n \t\t */\n \t\tsep_char = '\\t';\n \t\teol_char = '\\n';\n-\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tpath = quote_path(it->string, s->prefix, &buf, 0);\n \t\tif (d->rename_source)\n-\t\t\tpath_from = quote_path(d->rename_source, s->prefix, &buf_from);\n+\t\t\tpath_from = quote_path(d->rename_source, s->prefix, &buf_from, 0);\n \t}\n \n \tif (path_from)\n@@ -2310,7 +2310,7 @@ static void wt_porcelain_v2_print_unmerged_entry(\n \tif (s->null_termination)\n \t\tpath_index = it->string;\n \telse\n-\t\tpath_index = quote_path(it->string, s->prefix, &buf_index);\n+\t\tpath_index = quote_path(it->string, s->prefix, &buf_index, 0);\n \n \tfprintf(s->fp, \"%c %s %s %06o %06o %06o %06o %s %s %s %s%c\",\n \t\t\tunmerged_prefix, key, submodule_token,\n@@ -2343,7 +2343,7 @@ static void wt_porcelain_v2_print_other(\n \t\tpath = it->string;\n \t\teol_char = '\\0';\n \t} else {\n-\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\tpath = quote_path(it->string, s->prefix, &buf, 0);\n \t\teol_char = '\\n';\n \t}\n \n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405334","messageId":"20200910170159.1278781-4-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 3/7] quote_path: optionally allow quoting a path with SP in it","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:55Z","receivedAt":"2020-09-10T17:03:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Some code in wt-status.c special case a path with SP in it, which\nusually does not have to be c-quoted, and ensure that such a path\ndoes get quoted. Move the logic to quote_path() and give it a bit\nin the flags word, QUOTE_PATH_QUOTE_SP.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n quote.c | 7 +++++++\n quote.h | 1 +\n wt-status.c | 15 +++------------\n 3 files changed, 11 insertions(+), 12 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex a86f9f22a2..aa9a37b1b1 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -360,6 +360,13 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n \tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n \tstrbuf_release(&sb);\n \n+\tif ((flags & QUOTE_PATH_QUOTE_SP) &&\n+\t (out->buf[0] != '\"' && strchr(out->buf, ' '))) {\n+\t\t/* Ensure the whole thing is quoted if the path has SP in it */\n+\t\tstrbuf_insertstr(out, 0, \"\\\"\");\n+\t\tstrbuf_addch(out, '\"');\n+\t}\n+\n \treturn out->buf;\n }\n \ndiff --git a/quote.h b/quote.h\nindex 4687b5daf4..1918d1e00e 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -73,6 +73,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \n /* quote path as relative to the given prefix */\n char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n+#define QUOTE_PATH_QUOTE_SP 01\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex d6ca7bd52c..adbf6958bd 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1877,21 +1877,12 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\tconst char *one;\n \n \t\tif (d->rename_source) {\n-\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf, 0);\n-\t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n-\t\t\t\tputchar('\"');\n-\t\t\t\tstrbuf_addch(&onebuf, '\"');\n-\t\t\t\tone = onebuf.buf;\n-\t\t\t}\n+\t\t\tone = quote_path(d->rename_source, s->prefix, &onebuf,\n+\t\t\t\t\t QUOTE_PATH_QUOTE_SP);\n \t\t\tprintf(\"%s -> \", one);\n \t\t\tstrbuf_release(&onebuf);\n \t\t}\n-\t\tone = quote_path(it->string, s->prefix, &onebuf, 0);\n-\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n-\t\t\tputchar('\"');\n-\t\t\tstrbuf_addch(&onebuf, '\"');\n-\t\t\tone = onebuf.buf;\n-\t\t}\n+\t\tone = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);\n \t\tprintf(\"%s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405335","messageId":"20200910170159.1278781-7-gitster@pobox.com","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"[PATCH v2 6/7] quote: rename misnamed sq_lookup[] to cq_lookup[]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T17:01:58Z","receivedAt":"2020-09-10T17:03:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This table is used to see if each byte needs quoting when responding\nto a request to C-quote the string, not quoting with single-quote in\nthe shell style. Similarly, sq_must_quote() is fed each byte from\nthe string being C-quoted.\n\nNo behaviour change intended.\n\nSigned-off-by: Junio C Hamano \n---\n quote.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex b8107cd403..9a6e0e7dea 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -210,7 +210,7 @@ int sq_dequote_to_strvec(char *arg, struct strvec *array)\n */\n #define X8(x) x, x, x, x, x, x, x, x\n #define X16(x) X8(x), X8(x)\n-static signed char const sq_lookup[256] = {\n+static signed char const cq_lookup[256] = {\n \t/* 0 1 2 3 4 5 6 7 */\n \t/* 0x00 */ 1, 1, 1, 1, 1, 1, 1, 'a',\n \t/* 0x08 */ 'b', 't', 'n', 'v', 'f', 'r', 1, 1,\n@@ -223,9 +223,9 @@ static signed char const sq_lookup[256] = {\n \t/* 0x80 */ /* set to 0 */\n };\n \n-static inline int sq_must_quote(char c)\n+static inline int cq_must_quote(char c)\n {\n-\treturn sq_lookup[(unsigned char)c] + quote_path_fully > 0;\n+\treturn cq_lookup[(unsigned char)c] + quote_path_fully > 0;\n }\n \n /* returns the longest prefix not needing a quote up to maxlen if positive.\n@@ -235,9 +235,9 @@ static size_t next_quote_pos(const char *s, ssize_t maxlen)\n {\n \tsize_t len;\n \tif (maxlen < 0) {\n-\t\tfor (len = 0; !sq_must_quote(s[len]); len++);\n+\t\tfor (len = 0; !cq_must_quote(s[len]); len++);\n \t} else {\n-\t\tfor (len = 0; len < maxlen && !sq_must_quote(s[len]); len++);\n+\t\tfor (len = 0; len < maxlen && !cq_must_quote(s[len]); len++);\n \t}\n \treturn len;\n }\n@@ -291,8 +291,8 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \t\tch = (unsigned char)*p++;\n \t\tif (maxlen >= 0)\n \t\t\tmaxlen -= len + 1;\n-\t\tif (sq_lookup[ch] >= ' ') {\n-\t\t\tEMIT(sq_lookup[ch]);\n+\t\tif (cq_lookup[ch] >= ' ') {\n+\t\t\tEMIT(cq_lookup[ch]);\n \t\t} else {\n \t\t\tEMIT(((ch >> 6) & 03) + '0');\n \t\t\tEMIT(((ch >> 3) & 07) + '0');\n-- \n2.28.0-603-ga98dad7d4d\n\n"},{"id":"405346","messageId":"20200910180814.GA1917207@coredump.intra.peff.net","threadId":"54206","inReplyTo":"20200910170159.1278781-5-gitster@pobox.com","subject":"Re: [PATCH v2 4/7] quote_path: code clarification","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T18:08:14Z","receivedAt":"2020-09-10T18:08:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 10, 2020 at 10:01:56AM -0700, Junio C Hamano wrote:\n\n> Instead we can scan the path upfront to see if the input has SP in\n> it. If so, we tell quote_c_style_counted() not to enclose its\n> output in a dq-pair, and we add a dq-pair ourselves. Whether the\n> input had bytes that quote_c_style_counted() uses backslash quoting,\n> this would give us a desired quoted string. If the input does not\n> have SP in it, we just let quote_c_style_counted() do its thing as\n> usual, which would enclose the result in a dq-pair only when needed.\n\nNice. I think the result is easier to follow than the original, not to\nmention more efficient. And the fact that we didn't need to touch\nquote_c_style_counted() is the cherry on top.\n\n> +\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n> [...]\n> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq);\n\nI think force_dq is already normalized to 0/1 by the &&, so we wouldn't\nneed the \"!!\" here.\n\n-Peff\n"},{"id":"405348","messageId":"20200910181300.GB1917207@coredump.intra.peff.net","threadId":"54206","inReplyTo":"20200910170159.1278781-6-gitster@pobox.com","subject":"Re: [PATCH v2 5/7] wt-status: consistently quote paths in \"status --short\" output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T18:13:00Z","receivedAt":"2020-09-10T18:13:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 10, 2020 at 10:01:57AM -0700, Junio C Hamano wrote:\n\n> Tracked paths with SP in them were cquoted in \"git status --short\"\n> output, but untracked, ignored, and unmerged paths weren't.\n\nBy the way, the long status output will quote paths as appropriate, but\n_not_ apply this \"we should quote spaces\" rule. I'm not sure if that\ncame up in earlier discussion or not. Certainly we're free to do\nwhatever looks nicest to humans there, so maybe we prefer to avoid\nquoting as much as possible.\n\n-Peff\n"},{"id":"405350","messageId":"xmqqft7ppjdk.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"20200910181300.GB1917207@coredump.intra.peff.net","subject":"Re: [PATCH v2 5/7] wt-status: consistently quote paths in \"status --short\" output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T18:38:31Z","receivedAt":"2020-09-10T18:40:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Sep 10, 2020 at 10:01:57AM -0700, Junio C Hamano wrote:\n>\n>> Tracked paths with SP in them were cquoted in \"git status --short\"\n>> output, but untracked, ignored, and unmerged paths weren't.\n>\n> By the way, the long status output will quote paths as appropriate, but\n> _not_ apply this \"we should quote spaces\" rule. I'm not sure if that\n> came up in earlier discussion or not. Certainly we're free to do\n> whatever looks nicest to humans there, so maybe we prefer to avoid\n> quoting as much as possible.\n\nYup, the long output format is strictly for human consumption and\none fewer case to enclose the path in a dq-pair is good ;-).\n\n"},{"id":"405351","messageId":"xmqqblidpj9x.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"20200910180814.GA1917207@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/7] quote_path: code clarification","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T18:40:42Z","receivedAt":"2020-09-10T18:42:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Sep 10, 2020 at 10:01:56AM -0700, Junio C Hamano wrote:\n>\n>> Instead we can scan the path upfront to see if the input has SP in\n>> it. If so, we tell quote_c_style_counted() not to enclose its\n>> output in a dq-pair, and we add a dq-pair ourselves. Whether the\n>> input had bytes that quote_c_style_counted() uses backslash quoting,\n>> this would give us a desired quoted string. If the input does not\n>> have SP in it, we just let quote_c_style_counted() do its thing as\n>> usual, which would enclose the result in a dq-pair only when needed.\n>\n> Nice. I think the result is easier to follow than the original, not to\n> mention more efficient. And the fact that we didn't need to touch\n> quote_c_style_counted() is the cherry on top.\n>\n>> +\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n>> [...]\n>> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq);\n>\n> I think force_dq is already normalized to 0/1 by the &&, so we wouldn't\n> need the \"!!\" here.\n\nTrue. CQUOTE_NODQ patch would get rid of it in the end, though ;-)\n"},{"id":"405366","messageId":"20200910192920.GA1920978@coredump.intra.peff.net","threadId":"54206","inReplyTo":"xmqqblidpj9x.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 4/7] quote_path: code clarification","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T19:29:20Z","receivedAt":"2020-09-10T19:33:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 10, 2020 at 11:40:42AM -0700, Junio C Hamano wrote:\n\n> >> +\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n> >> [...]\n> >> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, !!force_dq);\n> >\n> > I think force_dq is already normalized to 0/1 by the &&, so we wouldn't\n> > need the \"!!\" here.\n> \n> True. CQUOTE_NODQ patch would get rid of it in the end, though ;-)\n\nOh, true. I didn't think to look ahead. Having done so, yeah, I don't\nsee any point in quibbling about this one. :)\n\n-Peff\n"},{"id":"405368","messageId":"xmqqo8mdr7ve.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"20200910122111.GA57925@coredump.intra.peff.net","subject":"Re: [PATCH 2/6] quote_path: give flags parameter to quote_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T15:04:05Z","receivedAt":"2020-09-10T19:35:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Sep 08, 2020 at 01:52:20PM -0700, Junio C Hamano wrote:\n>\n>> -char *quote_path(const char *in, const char *prefix, struct strbuf *out)\n>> +char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags)\n>> {\n>> \tstruct strbuf sb = STRBUF_INIT;\n>> \tconst char *rel = relative_path(in, prefix, &sb);\n>> \tstrbuf_reset(out);\n>> -\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n>> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n>> \tstrbuf_release(&sb);\n>\n> Here we take \"flags\", but then we pass it along in place of\n> quote_c_style_counted()'s \"no_dq\" parameter. That seems unlikely to be\n> the right thing (and indeed, it's reverted in the next commit).\n\nYou are seeing a rebase artifact. It needs to be corrected. Thanks\nfor noticing.\n\nAs you seem to have guessed correctly, the initial plan was to have\nthe new logic in quote-c-style-counted and make anything at higher\nlevel of the callchain just relay the \"SP must be quoted\" bit, and\nthe final two patches that are no longer necessary in the current\nseries were placed much earlier in an earlier draft of the series\nas preparatory steps for that endgame.\n\nBut it turned out that the main loop of quote-c-style-counted needs\na surgery that is a bit larger than I would have liked, so the final\nseries consumes the \"SP must be quoted\" bit in quote_path() without\ntelling quote-c-style-counted about it.\n\nThe problem with the main loop of quote-style-counted I saw was that\nthe next_quote_pos() is designed to return the position of the byte\nthat must be prefixed with a backslash, and the machinery to help it\n(namely, cq_must_quote() and cq_lookup[]) are written with that in\nmind quite firmly. The handling of SP we would be optionally adding\nto the mix is somewhat different---it forces the end result to be\nenclosed within a dq-pair, but it byitself does not need backslash\nquoting. Which means cq_lookup[] table needs to somehow encode a\nbit per byte that says \"byte with this value itself does not need to\nbe backslash-quoted, but the entire thing needs to be placed in a\ndq-pair\", and/or next_quote_pos() needs be able to say \"here is a\nbyte to cause us to enclose the whole thing in a dq-pair, but the\nbyte itself need not be backslash-quoted\".\n\nOf course none of the above becomes unnecessary if we scan the whole\nstring for SP before the main loop in quote-c-style-counted, but the\nfunction was written to process the input in a single pass and such\na change defeats its design. If we need to do it in two passes, we\ncan have the caller do so anyway, at least for now. That thinking\nlead to the final organization of the series, with two steps that\nused to be preparatory for passing the flag down thru to the bottom\nlayer rebased out as a discardable appendix at the end.\n\nThanks.\n"},{"id":"405371","messageId":"xmqq7dt1r78z.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"xmqqo8mdr7ve.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/6] quote_path: give flags parameter to quote_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T15:17:32Z","receivedAt":"2020-09-10T19:42:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Of course none of the above becomes unnecessary if we scan the whole\n> string for SP before the main loop in quote-c-style-counted, but the\n> function was written to process the input in a single pass and such\n> a change defeats its design. If we need to do it in two passes, we\n> can have the caller do so anyway, at least for now. That thinking\n> lead to the final organization of the series, with two steps that\n> used to be preparatory for passing the flag down thru to the bottom\n> layer rebased out as a discardable appendix at the end.\n\nActually, this made me realize that another variant is possible.\nIt might be easier to read, or it might not. Since I cannot tell\nwithout actually writing one, let's see ...\n\nInstead of scanning the output from quote-c-style-counted for SP in\nthe caller, the caller can do something like this.\n\n/* quote path as relative to the given prefix */\nchar *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags)\n{\n\tstruct strbuf sb = STRBUF_INIT;\n\tconst char *rel = relative_path(in, prefix, &sb);\n\tint add_dq_ourselves = !!(flags & QUOTE_PATH_QUOTE_SP) && !!strchr(rel, ' ');\n\n\tstrbuf_reset(out);\n if (add_dq_ourselves)\n \tstrbuf_addch(out, '\"');\n\tquote_c_style_counted(rel, strlen(rel), out, NULL,\n \t\t add_dq_ourselves ? CQUOTE_NODQ : 0);\n if (add_dq_ourselves)\n \tstrbuf_addch(out, '\"');\n\tstrbuf_release(&sb);\n\n\treturn out->buf;\n}\n\nThat is, we tell quote-c-style-counted not to put dq-pair around its\noutput whether it needs to do the backslash quoting, if we know we\nwant the result inside dq-pair anyway (iow, when the caller wants us\nto treat SP specially and we do see SP in the input).\n\nI don't know if this is easier to follow or not. I do think so\nright now but that is only because it is still fresh in my brain.\n"},{"id":"405372","messageId":"xmqq363pr6r6.fsf@gitster.c.googlers.com","threadId":"54206","inReplyTo":"b48b2be0-c417-9140-54f1-73d0c59c40d5@web.de","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-10T15:28:13Z","receivedAt":"2020-09-10T19:45:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n>>> +\tstruct strbuf sb = STRBUF_INIT;\n>>> +\tconst char *rel = relative_path(in, prefix, &sb);\n>>> +\tint need_quotes = *rel != '\"' && strchr(rel, ' ');\n>>\n>> relative_path() does not quote, so \"begins with a dq\" is not a good\n>> test to see \"if we were to pass this string to quote_c_style(), would\n>> we get it back quoted already so we won't have to surround the\n>> result with an extra pair of dq ourselves?\".\n>\n> Ha!, that's true. Makes me wonder how it was still able to pass the\n> test suite, though..\n\nThe logic to refrain from ading extra dq-pair around the output from\nquote-c-style is \"if it is already in a dq-pair, don't bother\". The\nbug will trigger only when the input string has a character that\nneeds to be quoted *and* SP in it. In such a case, relative_path()\nis likely to return *rel != '\"' and you declare \"we need to add a\ndq-pair ourselves\", quote-c-style would give us a result in a\ndq-pair, and you add one extra layer. \n\nBut without a test with a path with both '\"' and ' ' in it, such a\nbug would not trigger. Brian's test only checks what happens to a\npath with SP in it, without any other funny characters that need\nquoting with quote-c-style.\n\n\n"},{"id":"405377","messageId":"20200910202655.GA1938792@coredump.intra.peff.net","threadId":"54206","inReplyTo":"xmqq7dt1r78z.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/6] quote_path: give flags parameter to quote_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T20:26:55Z","receivedAt":"2020-09-10T20:49:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 10, 2020 at 08:17:32AM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Of course none of the above becomes unnecessary if we scan the whole\n> > string for SP before the main loop in quote-c-style-counted, but the\n> > function was written to process the input in a single pass and such\n> > a change defeats its design. If we need to do it in two passes, we\n> > can have the caller do so anyway, at least for now. That thinking\n> > lead to the final organization of the series, with two steps that\n> > used to be preparatory for passing the flag down thru to the bottom\n> > layer rebased out as a discardable appendix at the end.\n> \n> Actually, this made me realize that another variant is possible.\n> It might be easier to read, or it might not. Since I cannot tell\n> without actually writing one, let's see ...\n\nVger seems to be delivering slowly and out-of-order the last day or two,\nso I got rather confused to receive this after seeing your v2. :)\n\n> I don't know if this is easier to follow or not. I do think so\n> right now but that is only because it is still fresh in my brain.\n\nI do think it is easier to read than the original.\n\nOne minor nit with your analysis, though: the current code is actually\ntwo-pass already. One pass finds the next quoted character, but then we\nhave to take another pass to copy it into place. That second pass can be\ndone with a memcpy(), which helps.\n\nIf you know you are quoting, you can do a true single-pass\ncharacter-by-character. But of course part of our task is to find out\n_if_ we are quoting. And even if that were not the case, on modern\nprocessors it is not always true that single-pass is going to be faster.\nThis code is definitely not such a hot-spot that it's worth doing that\nkind of micro-optimization.\n\n Aside: We _do_ have spots where that is not true. When I looked at\n replacing xdiff's hash the sticking point was that we compute the\n newlines _and_ hash in a single pass. Most \"fast\" hash functions are\n optimized to take bigger sequences of data, but splitting out the\n newline-finding eliminated any gains.\n\n-Peff\n\n\n"},{"id":"405382","messageId":"b48b2be0-c417-9140-54f1-73d0c59c40d5@web.de","threadId":"54206","inReplyTo":"xmqqa6xytwtn.fsf@gitster.c.googlers.com","subject":"Re: [Bug report] git status doesn't escape paths of untracked files","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-09-10T14:23:02Z","receivedAt":"2020-09-10T21:24:25Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 10.09.20 um 00:22 schrieb Junio C Hamano:\n> René Scharfe writes:\n>> diff --git a/wt-status.c b/wt-status.c\n>> index ff43932402..e0711aff04 100644\n>> --- a/wt-status.c\n>> +++ b/wt-status.c\n>> @@ -1829,19 +1829,19 @@ static void wt_longstatus_print(struct wt_status *s)\n>> \t\twt_longstatus_print_stash_summary(s);\n>> }\n>>\n>> -static void print_cquoted(const char *fmt, const char *path, const char *prefix)\n>> +static const char *cquote(const char *in, const char *prefix, struct strbuf *out)\n>> {\n>> -\tstruct strbuf onebuf = STRBUF_INIT;\n>> -\tconst char *one;\n>> -\n>> -\tone = quote_path(path, prefix, &onebuf);\n>> -\tif (*one != '\"' && strchr(one, ' ')) {\n>> -\t\tstrbuf_insertstr(&onebuf, 0, \"\\\"\");\n>> -\t\tstrbuf_addch(&onebuf, '\"');\n>> -\t\tone = onebuf.buf;\n>> -\t}\n>> -\tprintf(fmt, one);\n>> -\tstrbuf_release(&onebuf);\n>> +\tstruct strbuf sb = STRBUF_INIT;\n>> +\tconst char *rel = relative_path(in, prefix, &sb);\n>> +\tint need_quotes = *rel != '\"' && strchr(rel, ' ');\n>\n> relative_path() does not quote, so \"begins with a dq\" is not a good\n> test to see \"if we were to pass this string to quote_c_style(), would\n> we get it back quoted already so we won't have to surround the\n> result with an extra pair of dq ourselves?\".\n\nHa!, that's true. Makes me wonder how it was still able to pass the\ntest suite, though..\n\n>\n>> +\tstrbuf_reset(out);\n>> +\tif (need_quotes)\n>> +\t\tstrbuf_addch(out, '\"');\n>> +\tquote_c_style(rel, out, NULL, 0);\n>> +\tif (need_quotes)\n>> +\t\tstrbuf_addch(out, '\"');\n>> +\tstrbuf_release(&sb);\n>> +\treturn out->buf;\n>> }\n"},{"id":"405385","messageId":"20200910123843.GC57925@coredump.intra.peff.net","threadId":"54206","inReplyTo":"20200908205224.4126551-7-gitster@pobox.com","subject":"Re: [PATCH 6/6] quote: turn 'nodq' parameter into a set of flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T12:38:43Z","receivedAt":"2020-09-10T21:54:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 08, 2020 at 01:52:24PM -0700, Junio C Hamano wrote:\n\n> quote_c_style() and its friend quote_two_c_style() both take an\n> optional \"please omit the double quotes around the quoted body\"\n> parameter. Turn it into a flag word, assign one bit out of it,\n> and call it CQUOTE_NODQ bit.\n\nSounds reasonable.\n\nIf this shared the same \"flags\" namespace as quote_path(), then we\nreally could pass quote_path() flags along. And in fact, that could be\nthe first step to just teaching quote_c_style() the \"if it has spaces,\nthen quote it\" rule. Maybe not worth spending more time on, but just\nthinking out loud after my earlier comments.\n\n> @@ -272,6 +272,7 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n> \t\tcount += (l); \\\n> \t} while (0)\n> \n> +\tint no_dq = !!(flags & CQUOTE_NODQ);\n> \tsize_t len, count = 0;\n> \tconst char *p = name;\n\nLooking at the context, I wondered how this was not adding a\ndecl-after-statement (and how we were not already complaining about the\nexisting ones). But the reason is that the code above it is part of a\nmacro.\n\n-Peff\n"},{"id":"405386","messageId":"20200910122111.GA57925@coredump.intra.peff.net","threadId":"54206","inReplyTo":"20200908205224.4126551-3-gitster@pobox.com","subject":"Re: [PATCH 2/6] quote_path: give flags parameter to quote_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-10T12:21:11Z","receivedAt":"2020-09-10T21:54:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 08, 2020 at 01:52:20PM -0700, Junio C Hamano wrote:\n\n> -char *quote_path(const char *in, const char *prefix, struct strbuf *out)\n> +char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> \tconst char *rel = relative_path(in, prefix, &sb);\n> \tstrbuf_reset(out);\n> -\tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, flags);\n> \tstrbuf_release(&sb);\n\nHere we take \"flags\", but then we pass it along in place of\nquote_c_style_counted()'s \"no_dq\" parameter. That seems unlikely to be\nthe right thing (and indeed, it's reverted in the next commit).\n\n> --- a/quote.h\n> +++ b/quote.h\n> @@ -72,7 +72,7 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n> \t\t\t\tFILE *fp, int terminator);\n> \n> /* quote path as relative to the given prefix */\n> -char *quote_path(const char *in, const char *prefix, struct strbuf *out);\n> +char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned);\n\nThe meaning of the last parameter is left a bit vague. :) Maybe worth\ncalling it \"unsigned flags\" (I don't mind omitting parameter names when\nthey are obvious, but I don't think this counts).\n\n-Peff\n"},{"id":"405389","messageId":"20200910230317.GK241078@camp.crustytoothpaste.net","threadId":"54206","inReplyTo":"20200910170159.1278781-1-gitster@pobox.com","subject":"Re: [PATCH v2 0/7] quote_path() clean-ups","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-09-10T23:03:17Z","receivedAt":"2020-09-10T23:03:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-09-10 at 17:01:52, Junio C Hamano wrote:\n> Here is an update, after seeing Peff's review.\n> \n> The overall structure of the series stays the same.\n> \n> * The second patch lost an incorrect use of 'flags' parameter down\n> to quote_c_style_counted() from quote_path(). In the function\n> declaration of quote_path(), the flags parameter is now named.\n> \n> * The third patch that moves the \"optionally quote path with SP in\n> it\" logic to quote_path() is essentialy unchanged.\n> \n> * But the fourth patch rewrites the implementation to avoid\n> shifting the output bytes with insertstr().\n> \n> * The fifth patch (formerly the fourth) that teachs \"status --short\"\n> to consistently quote hasn't changed, except that we test also\n> for the ignored paths.\n> \n> * The last two patches are unchanged. They are optional clean-ups\n> that are not required.\n> \n> We might want to add a bit more tests for:\n> \n> - how conflicted \"funny\" paths are shown.\n> \n> - how doubly funny paths (those that quote_c_style_counted() needs\n> to use backslash quoting *and* that contain SP) are shown.\n> \n> but I'd say that is outside the scope of this round. Seeing what is\n> done in t3300, the latter test cannot be written portably as far as\n> I can tell.\n\nI took a look and this seems like a great improvement. Thanks for\npicking up where my patch left off.\n-- \nbrian m. carlson: Houston, Texas, US\n"}]}