pretty: use format_commit_context.auto_color as colorbool
When we see "%C(auto)" as a format placeholder, we evaluate the "color" field of our pretty_print_context to decide whether we want color. The auto_color field of format_commit_context then stores the boolean result of want_color(), telling us the yes/no of whether we want color. But the resulting field is passed to various functions which expect a git_colorbool, like diff_get_color(), that will then pass it to want_color() again. It's not wrong to do so, since want_color() is idempotent. But it makes it harder to reason about the types, since we sometimes confuse colorbools and strict booleans. Let's instead store auto_color as the original colorbool itself. We'll have to make sure it is passed through want_color() when it is evaluated, but there is only one such spot (right next to where we assign it!). Every other caller just ends up passing it to get diff_get_color() either directly or through another helper. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
committed by
Junio C Hamano
parent
955000d917
commit
5e9ddd3c06
4
pretty.c
4
pretty.c
@@ -1455,8 +1455,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
|
|||||||
switch (placeholder[0]) {
|
switch (placeholder[0]) {
|
||||||
case 'C':
|
case 'C':
|
||||||
if (starts_with(placeholder + 1, "(auto)")) {
|
if (starts_with(placeholder + 1, "(auto)")) {
|
||||||
c->auto_color = want_color(c->pretty_ctx->color);
|
c->auto_color = c->pretty_ctx->color;
|
||||||
if (c->auto_color && sb->len)
|
if (want_color(c->auto_color) && sb->len)
|
||||||
strbuf_addstr(sb, GIT_COLOR_RESET);
|
strbuf_addstr(sb, GIT_COLOR_RESET);
|
||||||
return 7; /* consumed 7 bytes, "C(auto)" */
|
return 7; /* consumed 7 bytes, "C(auto)" */
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
Reference in New Issue
Block a user