[PATCH 0/13] unraveling the mysteries of color variables
- From
Jeff King <peff@peff.net>
- Date
- Sep 16, 2025, 20:10 UTC
- Message-ID
- <20250916201036.GA612463@coredump.intra.peff.net>
While reviewing the patches that were applied as jk/add-i-patch, Patrick noted that we could be using GIT_COLOR_UNKNOWN in more places. I punted there because I knew it would be a bit of a rabbit hole, and I think the patch count here perhaps justifies that view. ;)
The good news is that my digging did uncover two minor bugs, which are fixed here in patches 2 and 3. And the series should improve the readability of the code by using named constants and appropriate types more consistently.
The latter part of the series untangles some of the oddities related to color variables. I think the results are more readable, but they aren't strictly necessary because of C's weak type system (and because we define our constants in a favorable way). The final patch is gross and should not be applied, but I included it here as it explains how I was able to find all of the oddities.
[01/13]: color: use GIT_COLOR_* instead of numeric constants [02/13]: color: return enum from git_config_colorbool() [03/13]: grep: don't treat grep_opt.color as a strict bool [04/13]: diff: simplify color_moved check when flushing [05/13]: diff: don't use diff_options.use_color as a strict bool [06/13]: diff: pass o->use_color directly to fill_metainfo() [07/13]: diff: stop passing ecbdata->use_color as boolean [08/13]: pretty: use format_commit_context.auto_color as colorbool [09/13]: color: use git_colorbool enum to type to store colorbools [10/13]: color: return bool from want_color() [11/13]: add-interactive: retain colorbool values longer [12/13]: config: store want_color() result in a separate bool [13/13]: color: convert git_colorbool into a struct
add-interactive.c | 25 ++++++++++---------- add-interactive.h | 4 ++-- advice.c | 2 +- builtin/add.c | 2 +- builtin/am.c | 4 ++-- builtin/branch.c | 2 +- builtin/clean.c | 2 +- builtin/commit.c | 4 ++-- builtin/config.c | 27 +++++++++++----------- builtin/grep.c | 2 +- builtin/log.c | 4 ++-- builtin/push.c | 2 +- builtin/range-diff.c | 3 ++- builtin/show-branch.c | 2 +- color.c | 30 ++++++++++++------------ color.h | 15 +++++++----- combine-diff.c | 2 +- diff.c | 54 ++++++++++++++++++++----------------------- diff.h | 5 ++-- grep.c | 4 ++-- grep.h | 4 ++-- log-tree.c | 4 ++-- log-tree.h | 2 +- parse-options-cb.c | 6 ++--- pretty.c | 12 +++++----- pretty.h | 3 ++- ref-filter.h | 4 ++-- sequencer.c | 2 +- sideband.c | 8 +++---- transport.c | 2 +- wt-status.c | 6 ++--- wt-status.h | 2 +- 32 files changed, 127 insertions(+), 123 deletions(-)
-Peff