Fix color handling in git add -i
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v3 Latest11 patches 3 repliesIn review
- v211 patches 21 repliesSuperseded
- v19 patches 8 repliesSuperseded
Changes in v3 from the cover letter
* 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.
Patches in v3 No applies recorded yet
Reviews trailers per patch
None of the 11 patches has a Reviewed-by or Acked-by.
| Patch | Jeff King |
|---|---|
| 1/11 | |
| 2/11 | |
| 3/11 | |
| 4/11 | |
| 5/11 | |
| 6/11 | Co-authored-by |
| 7/11 | |
| 8/11 | |
| 9/11 | |
| 10/11 | |
| 11/11 | Co-authored-by |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH v3 00/11] Fix color handling in git add -i
This patch series started out as a tiny fix for a bug reported by Philippe Blain in
https://lore.kernel.org/git/[email protected]/.
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.
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 <[email protected]>
## 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.
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
Signature · 1 line
gitgitgadget