On Fri, Aug 21, 2015 at 10:43:58AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > I wonder if we can do this instead
> >
> > if (!omit_values) {
> > - if (show_keys)
> > + if (show_keys && value_)
> > strbuf_addch(buf, key_delim);
> >
> > though. That would eliminate the need for rolling back.
>
> No we cannot. "config --bool --get-regexp" could get a NULL value_
> and has to turn it to " true".
>
> Sorry for the noise.
Right, I had the same thought and reached the same conclusion.
By the way, I do not think the rollback by itself is gross, it is just
that it has to reproduce the "show_keys" logic. That is, it _really_
wants to be:
else {
/* nothing to show; rollback delim */
if (we_added_delim)
strbuf_setlen(buf, buf->len - 1);
}
and I use "show_keys" as a proxy for "did we add it". Which is
reproducing the logic that you quoted above, and if the two ever get out
of sync, it will be a bug. So you could write it as:
int we_added_delim = 0;
if (show_keys) {
strbuf_addch(buf, key_delim);
we_added_delim = 1;
}
I didn't, because it's ugly, and the function is short enough and
unlikely enough to change that it probably won't matter.
You could perhaps also write it as:
size_t baselen = buf->len;
if (show_keys)
strbuf_addch(buf, key_delim);
...
else {
/* rollback delim */
strbuf_setlen(buf, baselen);
}
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html