All of lore.kernel.org
 help / color / mirror / Atom feed
From: Derrick Stolee <derrickstolee@github.com>
To: phillip.wood@dunelm.org.uk,
	Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: gitster@pobox.com, johannes.schindelin@gmx.de, Jeff King <peff@peff.net>
Subject: Re: [PATCH v2] add: check color.ui for interactive add
Date: Wed, 7 Jun 2023 09:33:58 -0400	[thread overview]
Message-ID: <4b0e8026-4372-67ea-356c-b8073a6da20e@github.com> (raw)
In-Reply-To: <a76a91a0-cefe-807f-f1f6-4a277d724630@gmail.com>

On 6/7/2023 7:09 AM, Phillip Wood wrote:
> On 06/06/2023 15:20, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>>       -+    *** Commands ***
>>       -+      1: [s]tatus      2: [u]pdate      3: [r]evert      4: [a]dd untracked
>>       -+      5: [p]atch      6: [d]iff      7: [q]uit      8: [h]elp
>>       -+    What now> status        - show paths with changes
>>       -+    update        - add working tree state to the staged set of changes
>>       -+    revert        - revert staged set of changes back to the HEAD version
>>       -+    patch         - pick hunks and update selectively
>>       -+    diff          - view diff between HEAD and index
>>       -+    add untracked - add contents of untracked files to the staged set of changes
>>       -+    *** Commands ***
>>       -+      1: [s]tatus      2: [u]pdate      3: [r]evert      4: [a]dd untracked
>>       -+      5: [p]atch      6: [d]iff      7: [q]uit      8: [h]elp
>>       -+    What now> Bye.
>>       -+    EOF
>>       -+    test_cmp expect actual
>>       ++    test_cmp actual.raw actual
>>        +'
> 
> I know Junio suggested this change, but I'm in two minds as to whether
> it is a good idea. The reason is that we're no-longer testing that we
> add "[]" around the text that would have been highlighted if color was
> enabled. That is with --color we print "1: status" with the "s" highlighted
> rather than "1: [s]tatus". So while the revised patch tests there is no
> color in the output, it does not test that we print the output correctly
> in that case.

This is a good point, and something that I somewhat expected to find as
an example check in the rest of the test script. I think the color would
be disabled if we didn't do force_color, even before this fix.

However, I'll double-check that by adding a separate test for just that
bit of the UI, keeping this color.ui=false test focused on the color side
of things.

Thanks,
-Stolee

  reply	other threads:[~2023-06-07 13:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05 19:42 [PATCH] add: check color.ui for interactive add Derrick Stolee via GitGitGadget
2023-06-06  1:01 ` Junio C Hamano
2023-06-06  1:05   ` Junio C Hamano
2023-06-06  2:13 ` Jeff King
2023-06-06 13:32   ` Derrick Stolee
2023-06-06 14:20 ` [PATCH v2] " Derrick Stolee via GitGitGadget
2023-06-07 11:09   ` Phillip Wood
2023-06-07 13:33     ` Derrick Stolee [this message]
2023-06-12 17:09     ` Junio C Hamano
2023-06-07 13:44   ` [Patch v2 2/2] add: test use of brackets when color is disabled Derrick Stolee
2023-06-07 15:31     ` Phillip Wood
2023-06-12 17:13       ` Junio C Hamano

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=4b0e8026-4372-67ea-356c-b8073a6da20e@github.com \
    --to=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.