Re: Bug - Status - Space in Filename
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 9, 2017, 03:26 UTC
- Message-ID
- <xmqqvaikjfoj.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <655aaa9d2abf4be1b6ade0574d88c999@EXMBX01B.bhphotovideo.local>
Joseph Strauss <josephst@bhphoto.com> writes:
Show 13 quoted lines
> I believe I have found a bug in the way git status -s lists filenames. > > According to the documentation: > The fields (including the ->) are separated from each other by a single space. If a filename contains whitespace or other nonprintable characters, that field will be quoted in the manner of a C string literal: surrounded by ASCII double quote (34) characters, and with interior special characters backslash-escaped. > > While this is true in most situations, it does not seem to apply to merge conflicts. When a file has merge conflicts I am getting the following: > $ git status -s > UU some/path/with space/in/the/name > M "another/path/with space/in/the/name " > > I found the same problem for the following versions: > . git version 2.15.0.windows.1 > . git version 2.10.0
Thanks.
wt_shortstatus_status() has this code:
if (s->null_termination) {
fprintf(stdout, "%s%c", it->string, 0);
if (d->head_path)
fprintf(stdout, "%s%c", d->head_path, 0);
} else {
struct strbuf onebuf = STRBUF_INIT;
const char *one;
if (d->head_path) {
one = quote_path(d->head_path, s->prefix, &onebuf);
if (*one != '"' && strchr(one, ' ') != NULL) {
putchar('"');
strbuf_addch(&onebuf, '"');
one = onebuf.buf;
}
printf("%s -> ", one);
strbuf_release(&onebuf);
}
one = quote_path(it->string, s->prefix, &onebuf);
if (*one != '"' && strchr(one, ' ') != NULL) {
putchar('"');
strbuf_addch(&onebuf, '"');
one = onebuf.buf;
}
printf("%s\n", one);
strbuf_release(&onebuf);
}But the corresponding part in wt_shortstatus_unmerged() reads like this:
if (s->null_termination) {
fprintf(stdout, " %s%c", it->string, 0);
} else {
struct strbuf onebuf = STRBUF_INIT;
const char *one;
one = quote_path(it->string, s->prefix, &onebuf);
printf(" %s\n", one);
strbuf_release(&onebuf);
}The difference in d->head_path part is dealing about renames, which is irrelevant for showing unmerged paths, but the key difference is that the _unmerged() forgets to add the enclosing "" around the result of quote_path().
It seems that wt_shortstatus_other() shares the same issue. Perhaps this will "fix" it?
Having said all that, the whole "quote path using c-style" was designed very deliberately to treat SP (but not other kind of whitespaces like HT) as something we do *not* have to quote (and more importantly, do not *want* to quote, as pathnames with SP in it are quite common for those who are used to "My Documents/" etc.), so it is arguably that what is broken and incorrect is the extra quoting with dq done in wt_shortstatus_status(). It of course is too late to change the rules this late in the game, though.
Perhaps a better approach is to refactor the extra quoting I moved to this emit_with_optional_dq() helper into quote_path() which currently is just aliased to quote_path_relative(). It also is possible that we may want to extend the "no_dq" parameter that is given to quote_c_style() helper from a boolean to a set of flag bits, and allow callers to request "I want SP added to the set of bytes that triggers the quoting".
wt-status.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/wt-status.c b/wt-status.c index bedef256ce..472d53d596 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1671,6 +1671,20 @@ static void wt_longstatus_print(struct wt_status *s) wt_longstatus_print_stash_summary(s); } +static const char *emit_with_optional_dq(const char *in, const char *prefix, + struct strbuf *out) +{ + const char *one; + + one = quote_path(in, prefix, out); + if (*one != '"' && strchr(one, ' ') != NULL) { + putchar('"'); + strbuf_addch(out, '"'); + one = out->buf; + } + return one; +} + static void wt_shortstatus_unmerged(struct string_list_item *it, struct wt_status *s) { @@ -1692,8 +1706,9 @@ static void wt_shortstatus_unmerged(struct string_list_item *it, } else { struct strbuf onebuf = STRBUF_INIT; const char *one; - one = quote_path(it->string, s->prefix, &onebuf); - printf(" %s\n", one); + putchar(' '); + one = emit_with_optional_dq(it->string, s->prefix, &onebuf); + printf("%s\n", one); strbuf_release(&onebuf); } } @@ -1720,21 +1735,11 @@ static void wt_shortstatus_status(struct string_list_item *it, struct strbuf onebuf = STRBUF_INIT; const char *one; if (d->head_path) { - one = quote_path(d->head_path, s->prefix, &onebuf); - if (*one != '"' && strchr(one, ' ') != NULL) { - putchar('"'); - strbuf_addch(&onebuf, '"'); - one = onebuf.buf; - } + one = emit_with_optional_dq(d->head_path, s->prefix, &onebuf); printf("%s -> ", one); strbuf_release(&onebuf); } - one = quote_path(it->string, s->prefix, &onebuf); - if (*one != '"' && strchr(one, ' ') != NULL) { - putchar('"'); - strbuf_addch(&onebuf, '"'); - one = onebuf.buf; - } + one = emit_with_optional_dq(it->string, s->prefix, &onebuf); printf("%s\n", one); strbuf_release(&onebuf); } @@ -1748,9 +1753,10 @@ static void wt_shortstatus_other(struct string_list_item *it, } else { struct strbuf onebuf = STRBUF_INIT; const char *one; - one = quote_path(it->string, s->prefix, &onebuf); color_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), "%s", sign); - printf(" %s\n", one); + putchar(' '); + one = emit_with_optional_dq(it->string, s->prefix, &onebuf); + printf("%s\n", one); strbuf_release(&onebuf); } }