On Thu, Nov 09, 2017 at 12:26:20PM +0900, Junio C Hamano wrote:
> 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.
Yeah, I think the original sin is using " -> " in the --porcelain output
(which just got it from --short). That should have been a HT all along
to match the rest of the diff code. The --porcelain=v2 format gets this
right.
> 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".
Yeah, I had the same thought upon digging into the code.
That said, if this is the only place that has this funny quoting, it may
not be worth polluting the rest of the code with the idea that quoting
spaces is a good thing to do.
> 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;
> +}
This puts part of its output to stdout, and the other part into a
strbuf. That works for these callers, but maybe just emitting the whole
thing to stdout would be less confusing (and I suspect would clean up
the callers a bit, who would not have to worry about the strbuf at all).
-Peff
