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
next prev parent reply other threads:[~2020-11-16 16:09 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
List information: http://vger.kernel.org/majordomo-info.html
* 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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).