Re: [PATCH 3/6] quote_path: optionally allow quoting a path with SP in it

From: Jeff King <peff@peff.net>
Date: Sep 10, 2020, 12:35 UTC
Message-ID: <20200910123509.GB57925@coredump.intra.peff.net>
In-Reply-To: <20200908205224.4126551-4-gitster@pobox.com>

On Tue, Sep 08, 2020 at 01:52:21PM -0700, Junio C Hamano wrote:

Show 6 quoted lines

> Some code in wt-status.c special case a path with SP in it, which
> usually does not have to be c-quoted, and ensure that such a path
> does get quoted.  Move the logic to quote_path() and give it a bit
> in the flags word, QUOTE_PATH_QUOTE_SP.
> 
> No behaviour change intended.

Sounds like a good direction.

Show 14 quoted lines

> @@ -357,9 +357,16 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne
>  	struct strbuf sb = STRBUF_INIT;
>  	const char *rel = relative_path(in, prefix, &sb);
>  	strbuf_reset(out);
> -	quote_c_style_counted(rel, strlen(rel), out, NULL, flags);
> +	quote_c_style_counted(rel, strlen(rel), out, NULL, 0);
>  	strbuf_release(&sb);
>  
> +	if ((flags & QUOTE_PATH_QUOTE_SP) &&
> +	    (out->buf[0] != '"' && strchr(out->buf, ' '))) {
> +		/* Ensure the whole thing is quoted if the path has SP in it */
> +		strbuf_insertstr(out, 0, "\"");
> +		strbuf_addch(out, '"');
> +	}

This might be premature optimization, but using insertstr() means we have to recopy the entire string. As a general rule these kind of "splice into an array" operations tickle my accidentally-quadratic spider sense. But I don't think that's the case here (because we'd do at most one insertstr() per string; the real sin would be inserting characters like that throughout the string).

So it may not be worth addressing. I do think it would be conceptually simpler if we could just tell quote_c_style_counted() to look for space characters, but it looks like doing that ends up pretty invasive.

-Peff