Re: Bug - Status - Space in Filename

From: Jeff King <peff@peff.net>
Date: Nov 10, 2017, 09:58 UTC
Message-ID: <20171110095808.igydpwweyceu6qcf@sigill.intra.peff.net>
In-Reply-To: <xmqqvaijhs5b.fsf@gitster.mtv.corp.google.com>

On Fri, Nov 10, 2017 at 09:52:16AM +0900, Junio C Hamano wrote:

Show 20 quoted lines

> > 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.
> 
> Sounds sane.  We can probably use a helper like this:
> 
> static char *quote_path_with_sp(const char *in, const char *prefix, struct strbuf *out)
> {
> 	const char dq = '"';
> 
> 	quote_path(in, prefix, out);
> 	if (out->buf[0] != dq && strchr(out->buf, ' ') != NULL) {
> 		strbuf_insert(out, 0, &dq, 1);
> 		strbuf_addch(out, dq);
> 	}
> 	return out->buf;
> }
> 
> which allows the current users like shortstatus_status() to become a
> lot shorter.

Are there callers who don't just print the result? If not, we could just always emit. That's slightly more efficient since it drops the expensive strbuf_insert (though there are already so many copies going on in quote_path_relative that it hardly matters). But it also drops the need for the caller to know about the strbuf at all.

Like:

diff --git a/wt-status.c b/wt-status.c
index 937a87bbd5..4f4706a6e2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1703,6 +1703,18 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,
 	}
 }
 
+static void emit_path(const char *in, const char *prefix)
+{
+	struct strbuf buf = STRBUF_INIT;
+	quote_path(in, prefix, &buf);
+	if (buf.buf[0] != '"' && strchr(buf.buf, ' ') != NULL) {
+		putchar('"');
+		strbuf_addch(&buf, '"');
+	}
+	fwrite(buf.buf, 1, buf.len, stdout);
+	strbuf_release(&buf);
+}
+
 static void wt_shortstatus_status(struct string_list_item *it,
 			 struct wt_status *s)
 {
@@ -1722,26 +1734,12 @@ static void wt_shortstatus_status(struct string_list_item *it,
 		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);
+			emit_path(d->head_path, s->prefix);
+			printf(" -> ");
 		}
-		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);
+		emit_path(it->string, s->prefix);
+		putchar('\n');
 	}
 }
 

Though really I am fine with any solution that puts this pattern into a
helper function rather than repeating it inline.

-Peff