All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 0/7] add -i: close some regression test gaps
Date: Fri, 06 Dec 2019 13:08:18 +0000	[thread overview]
Message-ID: <pull.172.git.1575637705.gitgitgadget@gmail.com> (raw)

While re-implementing git add -i and git add -p in C, I tried to make sure
that there is test coverage for all of the features I convert from Perl to
C, to give me some confidence in the correctness from running the test suite
both with GIT_TEST_ADD_I_USE_BUILTIN=true and with 
GIT_TEST_ADD_I_USE_BUILTIN=false.

However, I discovered that there are a couple of gaps. This patch series
intends to close them.

The first patch might actually not be considered a gap by some: it basically
removes the need for the TTY prerequisite in the git add -i tests to verify
that the output is colored all right. This change is rather crucial for me,
though: on Windows, where the conversion to a built-in shows the most
obvious benefits, there are no pseudo terminals (yet), therefore git.exe 
cannot work with them (even if the MSYS2 Perl interpreter used by Git for
Windows knows about some sort of pty emulation). And I really wanted to make
sure that the colors work on Windows, as I personally get a lot out of those
color cues.

The patch series ends by addressing two issues that are not exactly covering
testing gaps:

 * While adding a test case, I noticed that git add -p exited with success 
   when it could not even generate a diff. This is so obviously wrong that I
   had to fix it right away (I noticed, actually, because my in-progress
   built-in git add -p failed, and the Perl version did not), and I used the
   same test case to verify that this is fixed once and for all.
   
   
 * While working on covering those test gaps, I noticed a problem in an
   early version of the built-in version of git add -p where the git apply
   --allow-overlap mode failed to work properly, for little reason, and I
   fixed it real quick.
   
   It would seem that the --allow-overlap function is not only purposefully
   under-documented, but also purposefully under-tested, probably to
   discourage its use. I do not quite understand the aversion to that
   option, but I did not feel like I should put up a battle here, so I did
   not accompany this fix with a new test script.
   
   In the end, the built-in version of git add -p does not use the 
   --allow-overlap function at all, anyway. Which should make everybody a
   lot happier.

Johannes Schindelin (7):
  t3701: add a test for advanced split-hunk editing
  t3701: avoid depending on the TTY prerequisite
  t3701: add a test for the different `add -p` prompts
  t3701: verify the shown messages when nothing can be added
  t3701: verify that the diff.algorithm config setting is handled
  git add -p: use non-zero exit code when the diff generation failed
  apply --allow-overlap: fix a corner case

 apply.c                    | 10 +++++
 git-add--interactive.perl  |  8 ++--
 t/t3701-add-interactive.sh | 90 ++++++++++++++++++++++++++++++++++----
 3 files changed, 97 insertions(+), 11 deletions(-)


base-commit: 2e697ced9d647d6998d70f010d582ba8019fe3af
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-172%2Fdscho%2Fadd-i-fixes-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-172/dscho/add-i-fixes-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/172
-- 
gitgitgadget

             reply	other threads:[~2019-12-06 13:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 13:08 Johannes Schindelin via GitGitGadget [this message]
2019-12-06 13:08 ` [PATCH 1/7] t3701: add a test for advanced split-hunk editing Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 2/7] t3701: avoid depending on the TTY prerequisite Johannes Schindelin via GitGitGadget
2019-12-16 12:18   ` SZEDER Gábor
2019-12-17  5:53     ` Jeff King
2019-12-06 13:08 ` [PATCH 3/7] t3701: add a test for the different `add -p` prompts Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 4/7] t3701: verify the shown messages when nothing can be added Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 5/7] t3701: verify that the diff.algorithm config setting is handled Johannes Schindelin via GitGitGadget
2019-12-06 13:08 ` [PATCH 6/7] git add -p: use non-zero exit code when the diff generation failed Johannes Schindelin via GitGitGadget
2019-12-06 20:48   ` Junio C Hamano
2019-12-06 13:08 ` [PATCH 7/7] apply --allow-overlap: fix a corner case Johannes Schindelin via GitGitGadget
2019-12-06 13:45   ` Junio C Hamano
2019-12-06 14:11     ` Johannes Schindelin
2019-12-06 16:40       ` Junio C Hamano
2019-12-06 17:56         ` Johannes Schindelin

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.172.git.1575637705.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /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.