public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: git@vger•kernel.org
Cc: Patrick Steinhardt <ps@pks•im>
Subject: [PATCH 0/13] unraveling the mysteries of color variables
Date: Tue, 16 Sep 2025 16:10:36 -0400	[thread overview]
Message-ID: <20250916201036.GA612463@coredump.intra.peff.net> (raw)

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

             reply	other threads:[~2025-09-16 20:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-16 20:10 Jeff King [this message]
2025-09-16 20:12 ` [PATCH 0/13] unraveling the mysteries of color variables Jeff King
2025-09-16 20:13 ` [PATCH 01/13] color: use GIT_COLOR_* instead of numeric constants Jeff King
2025-09-16 20:14 ` [PATCH 02/13] color: return enum from git_config_colorbool() Jeff King
2025-09-16 20:16 ` [PATCH 03/13] grep: don't treat grep_opt.color as a strict bool Jeff King
2025-09-16 20:17 ` [PATCH 04/13] diff: simplify color_moved check when flushing Jeff King
2025-09-16 20:19 ` [PATCH 05/13] diff: don't use diff_options.use_color as a strict bool Jeff King
2025-09-16 20:20 ` [PATCH 06/13] diff: pass o->use_color directly to fill_metainfo() Jeff King
2025-09-16 20:21 ` [PATCH 07/13] diff: stop passing ecbdata->use_color as boolean Jeff King
2025-09-16 20:22 ` [PATCH 08/13] pretty: use format_commit_context.auto_color as colorbool Jeff King
2025-09-16 20:24 ` [PATCH 09/13] color: use git_colorbool enum to type to store colorbools Jeff King
2025-09-16 23:13   ` [PATCH v1.5 9/13] color: use git_colorbool enum " Jeff King
2025-09-16 20:25 ` [PATCH 10/13] color: return bool from want_color() Jeff King
2025-09-16 20:26 ` [PATCH 11/13] add-interactive: retain colorbool values longer Jeff King
2025-09-16 20:26 ` [PATCH 12/13] config: store want_color() result in a separate bool Jeff King
2025-09-16 20:27 ` [PATCH 13/13 DO NOT APPLY] color: convert git_colorbool into a struct Jeff King
2025-09-16 20:35   ` Junio C Hamano
2025-09-16 20:28 ` [PATCH 0/13] unraveling the mysteries of color variables Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250916201036.GA612463@coredump.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=git@vger$(echo .)kernel.org \
    --cc=ps@pks$(echo .)im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox