diff mbox series

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

Message ID 20200908205224.4126551-4-gitster@pobox.com (mailing list archive)
State Superseded
Headers show
Series quote_path() clean-ups | expand

Commit Message

Junio C Hamano Sept. 8, 2020, 8:52 p.m. UTC
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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 quote.c     |  9 ++++++++-
 quote.h     |  1 +
 wt-status.c | 15 +++------------
 3 files changed, 12 insertions(+), 13 deletions(-)

Comments

Jeff King Sept. 10, 2020, 12:35 p.m. UTC | #1
On Tue, Sep 08, 2020 at 01:52:21PM -0700, Junio C Hamano wrote:

> 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.

> @@ -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
diff mbox series

Patch

diff --git a/quote.c b/quote.c
index 03ca85afb0..aa9a37b1b1 100644
--- a/quote.c
+++ b/quote.c
@@ -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, '"');
+	}
+
 	return out->buf;
 }
 
diff --git a/quote.h b/quote.h
index db7140b3c7..823f56cb94 100644
--- a/quote.h
+++ b/quote.h
@@ -73,6 +73,7 @@  void write_name_quoted_relative(const char *name, const char *prefix,
 
 /* quote path as relative to the given prefix */
 char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned);
+#define QUOTE_PATH_QUOTE_SP 01
 
 /* quoting as a string literal for other languages */
 void perl_quote_buf(struct strbuf *sb, const char *src);
diff --git a/wt-status.c b/wt-status.c
index d6ca7bd52c..adbf6958bd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1877,21 +1877,12 @@  static void wt_shortstatus_status(struct string_list_item *it,
 		const char *one;
 
 		if (d->rename_source) {
-			one = quote_path(d->rename_source, s->prefix, &onebuf, 0);
-			if (*one != '"' && strchr(one, ' ') != NULL) {
-				putchar('"');
-				strbuf_addch(&onebuf, '"');
-				one = onebuf.buf;
-			}
+			one = quote_path(d->rename_source, s->prefix, &onebuf,
+					 QUOTE_PATH_QUOTE_SP);
 			printf("%s -> ", one);
 			strbuf_release(&onebuf);
 		}
-		one = quote_path(it->string, s->prefix, &onebuf, 0);
-		if (*one != '"' && strchr(one, ' ') != NULL) {
-			putchar('"');
-			strbuf_addch(&onebuf, '"');
-			one = onebuf.buf;
-		}
+		one = quote_path(it->string, s->prefix, &onebuf, QUOTE_PATH_QUOTE_SP);
 		printf("%s\n", one);
 		strbuf_release(&onebuf);
 	}