Re: [Bug report] git status doesn't escape paths of untracked files

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 9, 2020, 22:22 UTC
Message-ID: <xmqqa6xytwtn.fsf@gitster.c.googlers.com>
In-Reply-To: <3a72c5f2-35cc-a865-d5f2-02706c48d8ec@web.de>

René Scharfe <l.s.r@web.de> writes:

> I don't particularly like how this print* function takes a printf
> format, but doesn't interpret its remaining arguments like printf does.

Yes. I think the more elaborate one that pushes the logic down to quote_path() I posted earlier is much cleaner in this regard.

Show 6 quoted lines

>>  - I am undecided if I like that the helper formats and also prints;
>>    I was hoping I can come up with a pure formatting helper that
>>    does not do I/O but it seems to be hard to arrange for the
>>    current callers.
>
> How about something like this then?

Thanks. I kind of like the fact that it is isolated compared to the "push the logic down to quote_path()" approach.

But it also is a double edged sword. It makes it harder for other codepaths to quote a path with SP in it by mistake, but if it turns out to be a good longer term thing to quote a path with SP in it, it makes it harder to do so.

Show 25 quoted lines

> diff --git a/wt-status.c b/wt-status.c
> index ff43932402..e0711aff04 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1829,19 +1829,19 @@ static void wt_longstatus_print(struct wt_status *s)
>  		wt_longstatus_print_stash_summary(s);
>  }
>
> -static void print_cquoted(const char *fmt, const char *path, const char *prefix)
> +static const char *cquote(const char *in, const char *prefix, struct strbuf *out)
>  {
> -	struct strbuf onebuf = STRBUF_INIT;
> -	const char *one;
> -
> -	one = quote_path(path, prefix, &onebuf);
> -	if (*one != '"' && strchr(one, ' ')) {
> -		strbuf_insertstr(&onebuf, 0, "\"");
> -		strbuf_addch(&onebuf, '"');
> -		one = onebuf.buf;
> -	}
> -	printf(fmt, one);
> -	strbuf_release(&onebuf);
> +	struct strbuf sb = STRBUF_INIT;
> +	const char *rel = relative_path(in, prefix, &sb);
> +	int need_quotes = *rel != '"' && strchr(rel, ' ');

relative_path() does not quote, so "begins with a dq" is not a good test to see "if we were to pass this string to quote_c_style(), would we get it back quoted already so we won't have to surround the result with an extra pair of dq ourselves?".

Show 9 quoted lines

> +	strbuf_reset(out);
> +	if (need_quotes)
> +		strbuf_addch(out, '"');
> +	quote_c_style(rel, out, NULL, 0);
> +	if (need_quotes)
> +		strbuf_addch(out, '"');
> +	strbuf_release(&sb);
> +	return out->buf;
>  }