Joseph Strauss <josephst@xxxxxxxxxxx> writes:
> 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);
}
}
