Jeff King <[email protected]> writes:
> When formatting a config value into a strbuf, we may end
> up stringifying it into a fixed-size buffer using sprintf,
> and then copying that buffer into the strbuf. We can
> eliminate the middle-man (and drop some calls to sprintf!)
> by writing directly to the strbuf.
>
> The reason it was written this way in the first place is
> that we need to know before writing the value whether to
> insert a delimiter. Instead of delaying the write of the
> value, we speculatively write the delimiter, and roll it
> back in the single case that cares.
>
> Signed-off-by: Jeff King <[email protected]>
> ---
> I admit the rollback is a little gross. The other option would be adding
> the delimiter in each of the conditional branches, which is also kind of
> nasty.
I actually am fine with this rollback. The "variable alone stands
for true" is not something a user can produce from the command line
very easily, so having to rollback is a rare event anyway.
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.
I briefly wondered how such a change would interact with
if (types == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
git_config_int64(key_, value_ ? value_ : ""));
that immediately follows it, but this "turn NULL into an empty
string" may be bogus in the first place, in the sense that
git_config_int64() should complain about a NULL value_ the same way
as it would complain about an empty string---both them are not an
integer. And indeed:
- git_parse_int64() that is called from git_config_int64() is
prepared to take both "" and NULL and return failure with EINVAL;
- die_bad_number() that is eventually called when parsing fails by
git_config_int64() is prepared to take NULL and turns it to an
empty string.
So perhaps we could do this squashed in?
builtin/config.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 71acc44..593b1ae 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -111,12 +111,12 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
if (show_keys)
strbuf_addstr(buf, key_);
if (!omit_values) {
- if (show_keys)
+ if (show_keys && value_)
strbuf_addch(buf, key_delim);
if (types == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
- git_config_int64(key_, value_ ? value_ :
""));
+ git_config_int64(key_, value_));
else if (types == TYPE_BOOL)
strbuf_addstr(buf, git_config_bool(key_, value_) ?
"true" : "false");
@@ -136,9 +136,8 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
} else if (value_) {
strbuf_addstr(buf, value_);
} else {
- /* Just show the key name; back out delimiter */
- if (show_keys)
- strbuf_setlen(buf, buf->len - 1);
+ /* Just show the key name */
+ ;
}
}
strbuf_addch(buf, term);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
