git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com>
Subject: Re: [PATCH v2 5/5] builtin/config: do not spawn pager when printing color codes
Date: Mon, 15 Sep 2025 10:28:23 -0700	[thread overview]
Message-ID: <xmqqikhjhbgo.fsf@gitster.g> (raw)
In-Reply-To: <20250915-pks-config-color-v2-5-e4290bd8d13c@pks.im> (Patrick Steinhardt's message of "Mon, 15 Sep 2025 14:52:52 +0200")

Patrick Steinhardt <ps@pks.im> writes:

> With `git config get --type=color` the user asks us to parse a specific
> configuration key and turn the value into an ANSI color escape sequence.
> The printed string can then for example be used as part of shell scripts
> to reuse the same colors as Git.
>
> Right now though we set up the auto-pager, which means that the string
> may instead be written to the pager command. This is of course quite
> nonsensical; there shouldn't be any use case where the color code should
> end up in the pager instead of in the TTY.
>
> Fix this by disabling the pager in case the user is asking us to print
> color sequences.

I am of two minds.  Part of me obviously agrees that it is more
straight forward with this change.  But it may 

An interactive user experimenting while writing their own script
might say something like

 $ git config --type=color --default="reverse red" n.n

If the command emitted directly to the terminal, then everything
they type from then on will be bloody red, but the pager protects
them from such an accident.  Instead, they are forced to say

 $ C=$(git config get --type=color --default="reverse red" n.n)
 $ R=$(git config get --type=color --default="reset" n.n)
 $ echo "So$C Bloody ${R}Red"

but these are likely what they would be writing in their script
anyway, so...

  reply	other threads:[~2025-09-15 17:28 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-11 13:24 [PATCH 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color" Patrick Steinhardt
2025-09-11 13:24 ` [PATCH 1/5] t1300: write test expectations in the test's body Patrick Steinhardt
2025-09-11 16:50   ` Kristoffer Haugsbakk
2025-09-15 11:41     ` Patrick Steinhardt
2025-09-11 13:24 ` [PATCH 2/5] t1300: small style fixups Patrick Steinhardt
2025-09-11 13:24 ` [PATCH 3/5] builtin/config: do not die in `get_color()` Patrick Steinhardt
2025-09-11 13:24 ` [PATCH 4/5] builtin/config: special-case retrieving colors without a key Patrick Steinhardt
2025-09-11 16:48   ` Kristoffer Haugsbakk
2025-09-15 11:41     ` Patrick Steinhardt
2025-09-11 13:24 ` [PATCH 5/5] builtin/config: do not spawn pager when printing color codes Patrick Steinhardt
2025-09-11 16:49   ` Kristoffer Haugsbakk
2025-09-15 12:52 ` [PATCH v2 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color" Patrick Steinhardt
2025-09-15 12:52   ` [PATCH v2 1/5] t1300: write test expectations in the test's body Patrick Steinhardt
2025-09-15 12:52   ` [PATCH v2 2/5] t1300: small style fixups Patrick Steinhardt
2025-09-15 12:52   ` [PATCH v2 3/5] builtin/config: do not die in `get_color()` Patrick Steinhardt
2025-09-15 12:52   ` [PATCH v2 4/5] builtin/config: special-case retrieving colors without a key Patrick Steinhardt
2025-09-15 17:19     ` Junio C Hamano
2025-09-15 12:52   ` [PATCH v2 5/5] builtin/config: do not spawn pager when printing color codes Patrick Steinhardt
2025-09-15 17:28     ` Junio C Hamano [this message]
2025-09-16  6:56       ` Kristoffer Haugsbakk
2025-09-18  6:03       ` Patrick Steinhardt
2025-09-18  6:14 ` [PATCH v3 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color" Patrick Steinhardt
2025-09-18  6:14   ` [PATCH v3 1/5] t1300: write test expectations in the test's body Patrick Steinhardt
2025-09-18  6:14   ` [PATCH v3 2/5] t1300: small style fixups Patrick Steinhardt
2025-09-18  6:14   ` [PATCH v3 3/5] builtin/config: do not die in `get_color()` Patrick Steinhardt
2025-09-18  6:14   ` [PATCH v3 4/5] builtin/config: special-case retrieving colors without a key Patrick Steinhardt
2025-09-18  6:49     ` Junio C Hamano
2025-09-22 13:04       ` Patrick Steinhardt
2025-09-22 16:29         ` Junio C Hamano
2025-09-18  6:14   ` [PATCH v3 5/5] builtin/config: do not spawn pager when printing color codes Patrick Steinhardt
2025-09-22 13:06 ` [PATCH v4 0/5] builtin/config: bug fixes for "get" subcommand with "--type=color" Patrick Steinhardt
2025-09-22 13:06   ` [PATCH v4 1/5] t1300: write test expectations in the test's body Patrick Steinhardt
2025-09-22 13:06   ` [PATCH v4 2/5] t1300: small style fixups Patrick Steinhardt
2025-09-22 13:06   ` [PATCH v4 3/5] builtin/config: do not die in `get_color()` Patrick Steinhardt
2025-09-22 13:06   ` [PATCH v4 4/5] builtin/config: special-case retrieving colors without a key Patrick Steinhardt
2025-09-22 13:06   ` [PATCH v4 5/5] builtin/config: do not spawn pager when printing color codes Patrick Steinhardt

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=xmqqikhjhbgo.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=ps@pks.im \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).