All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
	Jeff King <peff@peff.net>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH v3 00/11] Fix color handling in git add -i
Date: Mon, 16 Nov 2020 16:08:21 +0000	[thread overview]
Message-ID: <pull.785.v3.git.1605542912.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.785.v2.git.1605097704.gitgitgadget@gmail.com>

This patch series started out as a tiny fix for a bug reported by Philippe
Blain in 
https://lore.kernel.org/git/313B8999-1E99-4695-A20D-E48840C30879@gmail.com/.
And then I only wanted to add a regression test to make sure that this does
not regress. And then I just wanted to ensure that it passes both with the
Perl version of git add -i as well as with the built-in version.

And in no time I was looking at a real patch series.

Changes since v2:

 * The commit messages of patches 7/11 and 9/11 now stress why we want to
   align the output of the Perl vs the built-in version so slavishly: to be
   able to validate both versions against prerecorded output.
 * A typo was fixed in the commit message of patch 10/11.

Changes since v1:

 * The regression test now actually exercises the re-coloring (that is the
   primary purpose of git add -p looking at the color.diff.* variables).
 * The way the built-in git add -p renders hunk headers of split hunks was
   aligned with how the Perl version does things.
 * We now consistently prefer color.diff.context over color.diff.plain, no
   matter whether using the Perl or the built-in version of git add -p.
 * The commit message for the regression test no longer confuses readers by
   mentioning dash.
 * The regression test was structured a bit better, first testing error
   message coloring, then the menu in git add -i and then the diff coloring
   in git add -p.

Johannes Schindelin (11):
  add -i (built-in): do show an error message for incorrect inputs
  add -i (built-in): send error messages to stderr
  add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk
    headers
  add -i: use `reset_color` consistently
  add -i (built-in): prevent the `reset` "color" from being configured
  add -i (built-in): use correct names to load color.diff.* config
  add -p (built-in): do not color the progress indicator separately
  add -i (built-in): use the same indentation as the Perl version
  add -i (Perl version): color header to match the C version
  add -p: prefer color.diff.context over color.diff.plain
  add -i: verify in the tests that colors can be overridden

 add-interactive.c          | 38 ++++++++++-------
 add-patch.c                | 25 +++++++-----
 git-add--interactive.perl  | 12 +++---
 t/t3701-add-interactive.sh | 84 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 32 deletions(-)


base-commit: e4d83eee9239207622e2b1cc43967da5051c189c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-785%2Fdscho%2Ffix-add-i-colors-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-785/dscho/fix-add-i-colors-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/785

Range-diff vs v2:

  1:  6152122c04 =  1:  6152122c04 add -i (built-in): do show an error message for incorrect inputs
  2:  068813912b =  2:  068813912b add -i (built-in): send error messages to stderr
  3:  98deb538d9 =  3:  98deb538d9 add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk headers
  4:  c857c44932 =  4:  c857c44932 add -i: use `reset_color` consistently
  5:  337b45cad8 =  5:  337b45cad8 add -i (built-in): prevent the `reset` "color" from being configured
  6:  dcd2ffc458 =  6:  dcd2ffc458 add -i (built-in): use correct names to load color.diff.* config
  7:  73b6d60a80 !  7:  6a68bc5511 add -p (built-in): do not color the progress indicator separately
     @@ Commit message
          add -p (built-in): do not color the progress indicator separately
      
          The Perl version of this command colors the progress indicator and the
     -    prompt message in one go, let's do the same in the built-in version.
     +    prompt message in one go.
     +
     +    Let's do the same in the built-in version so that the same upcoming test
     +    (which will compare the output of `git add -p` against a known-good
     +    version) will pass both for the Perl version as well as for the built-in
     +    version.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
  8:  91ded2fbbe =  8:  168891f9f8 add -i (built-in): use the same indentation as the Perl version
  9:  304614751e !  9:  094a4ad90c add -i (Perl version): include indentation in the colored header
     @@ Metadata
      Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
      
       ## Commit message ##
     -    add -i (Perl version): include indentation in the colored header
     +    add -i (Perl version): color header to match the C version
      
     -    The header is formatted by padding each column heading with spaces up to
     -    the length of 12 characters. These padding spaces are naturally included
     -    when coloring the entire header.
     -
     -    However, the preceding five spaces indentation for non-flat lists were
     -    _not_ included in the Perl version, but _were_ included in the built-in
     -    version. Let's adjust the former to align with the latter's behavior.
     +    Both versions of `add -i` indent non-flat lists by five spaces. However
     +    when using color the C version prints these spaces after the ANSI color
     +    codes whereas the Perl version prints them before the color codes.
     +    Change the Perl version to match the C version to allow for introducing
     +    a test that verifies that both versions produce the exact same output.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
 10:  48d8e0badf ! 10:  9a4d2a33b5 add -p: prefer color.diff.context over color.diff.plain
     @@ Commit message
          the hard-coded default (which in this case is simply the empty string,
          as context lines are typically rendered without colored).
      
     -    This still needs to inconsistencies when both config names are used: the
     +    This still leads to inconsistencies when both config names are used: the
          initial diff will be colored by the diff machinery. Once edited by a
          user, a hunk has to be re-colored by `git add -p`, though, which would
          then use the other setting to color the context lines.
 11:  c94abff72f = 11:  492f46833a add -i: verify in the tests that colors can be overridden

-- 
gitgitgadget

  parent reply	other threads:[~2020-11-16 16:08 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10 23:42 [PATCH 0/9] Fix color handling in git add -i Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 1/9] add -i (built-in): do show an error message for incorrect inputs Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 2/9] add -i (built-in): send error messages to stderr Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 3/9] add -i: use `reset_color` consistently Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 4/9] add -i (built-in): prevent the `reset` "color" from being configured Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 5/9] add -i (built-in): use correct names to load color.diff.* config Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 6/9] add -p (built-in): do not color the progress indicator separately Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 7/9] add -i (built-in): use the same indentation as the Perl version Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 8/9] add -i (Perl version): include indentation in the colored header Johannes Schindelin via GitGitGadget
2020-11-10 23:42 ` [PATCH 9/9] add -i: verify in the tests that colors can be overridden Johannes Schindelin via GitGitGadget
2020-11-11  2:35   ` Jeff King
2020-11-11 15:53     ` Johannes Schindelin
2020-11-11 18:07       ` Jeff King
2020-11-12 14:04         ` Johannes Schindelin
2020-11-12 18:29           ` Jeff King
2020-11-15 23:35             ` Johannes Schindelin
2020-11-17  1:44               ` Jeff King
2020-11-11  2:36 ` [PATCH 0/9] Fix color handling in git add -i Jeff King
2020-11-11 12:28 ` [PATCH v2 00/11] " Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 01/11] add -i (built-in): do show an error message for incorrect inputs Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 02/11] add -i (built-in): send error messages to stderr Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 03/11] add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk headers Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 04/11] add -i: use `reset_color` consistently Johannes Schindelin via GitGitGadget
2020-11-13 12:03     ` Phillip Wood
2020-11-13 13:53       ` Johannes Schindelin
2020-11-13 16:04         ` Phillip Wood
2020-11-15 23:47           ` Johannes Schindelin
2020-11-11 12:28   ` [PATCH v2 05/11] add -i (built-in): prevent the `reset` "color" from being configured Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 06/11] add -i (built-in): use correct names to load color.diff.* config Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 07/11] add -p (built-in): do not color the progress indicator separately Johannes Schindelin via GitGitGadget
2020-11-13 12:07     ` Phillip Wood
2020-11-13 13:57       ` Johannes Schindelin
2020-11-13 16:06         ` Phillip Wood
2020-11-15 23:55           ` Johannes Schindelin
2020-11-11 12:28   ` [PATCH v2 08/11] add -i (built-in): use the same indentation as the Perl version Johannes Schindelin via GitGitGadget
2020-11-11 12:28   ` [PATCH v2 09/11] add -i (Perl version): include indentation in the colored header Johannes Schindelin via GitGitGadget
2020-11-13 12:11     ` Phillip Wood
2020-11-13 13:58       ` Johannes Schindelin
2020-11-13 16:12         ` Phillip Wood
2020-11-15 23:51           ` Johannes Schindelin
2020-11-11 12:28   ` [PATCH v2 10/11] add -p: prefer color.diff.context over color.diff.plain Johannes Schindelin via GitGitGadget
2020-11-13 14:04     ` Philippe Blain
2020-11-15 23:57       ` Johannes Schindelin
2020-11-13 14:08     ` Phillip Wood
2020-11-16  0:02       ` Johannes Schindelin
2020-11-11 12:28   ` [PATCH v2 11/11] add -i: verify in the tests that colors can be overridden Johannes Schindelin via GitGitGadget
2020-11-11 18:45   ` [PATCH v2 00/11] Fix color handling in git add -i Jeff King
2020-11-12 14:20     ` Johannes Schindelin
2020-11-12 18:40       ` Jeff King
2020-11-15 23:40         ` Johannes Schindelin
2020-11-17  1:49           ` Jeff King
2020-11-16 16:08   ` Johannes Schindelin via GitGitGadget [this message]
2020-11-16 16:08     ` [PATCH v3 01/11] add -i (built-in): do show an error message for incorrect inputs Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 02/11] add -i (built-in): send error messages to stderr Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 03/11] add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk headers Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 04/11] add -i: use `reset_color` consistently Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 05/11] add -i (built-in): prevent the `reset` "color" from being configured Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 06/11] add -i (built-in): use correct names to load color.diff.* config Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 07/11] add -p (built-in): do not color the progress indicator separately Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 08/11] add -i (built-in): use the same indentation as the Perl version Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 09/11] add -i (Perl version): color header to match the C version Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 10/11] add -p: prefer color.diff.context over color.diff.plain Johannes Schindelin via GitGitGadget
2020-11-16 16:08     ` [PATCH v3 11/11] add -i: verify in the tests that colors can be overridden Johannes Schindelin via GitGitGadget
2020-11-17  1:51     ` [PATCH v3 00/11] Fix color handling in git add -i Jeff King
2020-11-17  2:05       ` Jeff King
2020-11-25 21:59       ` 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=pull.785.v3.git.1605542912.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood123@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 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.