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

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

Reply via email to