Valid points, I don't think I have any objections to anything listed.

Would it be recommended to update to test_grep (and test_config from
previous message) in the same test files whilst I'm at it?

Thanks for the review :)

On Tue, 6 May 2025 at 01:02, Eric Sunshine [off-list ref] wrote:

On Mon, May 5, 2025 at 5:19 AM Leon Michalak via GitGitGadget
[off-list ref] wrote:

quoted

This teaches `add/commit --interactive` a new "context" subcommand, which
changes the amount of context lines subsequent subcommands like "patch"
or "diff" generate in their diffs.

Signed-off-by: Leon Michalak <redacted>
---

diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc

@@ -265,14 +265,15 @@ and type return, like this:
 ------------
     *** Commands ***
       1: status       2: update       3: revert       4: add untracked
-      5: patch        6: diff         7: quit         8: help
+      5: patch        6: diff         7: context      8: quit
+      9: help
     What now> 1

I'm not a `git add/commit --interactive' user, but I can imagine that
inserting "context" at 7 and bumping "quit" and "help" to 8 and 9,
respectively, is going to play havoc with muscle memory people have
built up over the years. To make this more friendly for existing
users, I'd suggest adding this new command at the end of the list
without changing the existing command numbers.

Also, looking at this list, I can't help but think that "context"
feels out of place among the other action-oriented commands. Moreover,
if --interactive mode grows more configuration/setting-like commands
in the future, do we really want to keep extending this menu for them?
Specifically, I'm wondering if it would instead make sense to
introduce a new item "9: settings" which takes the user to a
"Settings" submenu from which the number of context lines can be set.

quoted

-The main command loop has 6 subcommands (plus help and quit).
+The main command loop has 7 subcommands (plus help and quit).

Since you're touching this anyhow, let's fix this maintenance burden
once and for all by writing more it generically, perhaps like this:

   The main command loop has several subcommands (plus help and quit).

quoted

+context::
+
+  This lets you change the amount of context lines shown in diffs that
+  the 'patch' and 'diff' subcommands generate.

s/amount/number/

quoted

diff --git a/add-interactive.c b/add-interactive.c

@@ -1061,6 +1118,8 @@ static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED,
+       color_fprintf_ln(stdout, s->help_color, "context       - %s",
+                        _("change how many context lines diffs are generated with"));

Perhaps:

    _("change the number of diff context lines"));

quoted

@@ -1087,6 +1146,16 @@ static void choose_prompt_help(struct add_i_state *s)
+static void choose_prompt_help_context(struct add_i_state *s)
+{
+       color_fprintf_ln(stdout, s->help_color, "%s",
+                        _("Prompt help:"));
+       color_fprintf_ln(stdout, s->help_color, "<n>        - %s",
+                        _("specify new context lines amount"));

Likewise:

    _("change number of diff context lines"));

quoted

+       color_fprintf_ln(stdout, s->help_color, "           - %s",
+                        _("(empty) finish selecting"));

"finish selecting" looks like a copy/paste error from elsewhere in
this source file. Perhaps you meant something like:

    _("(empty) don't change number of context lines"));

quoted

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh

@@ -1230,4 +1237,23 @@ test_expect_success 'hunk splitting works with diff.suppressBlankEmpty' '
+test_expect_success 'change context works' '
+       git reset --hard &&
+       cat >template <<-\EOF &&
+       firstline
+       preline
+       TARGET
+       postline
+       lastline
+       EOF
+       sed "/TARGET/d" >x <template &&
+       git update-index --add x &&
+       git commit -m initial &&
+       sed "s/TARGET/ADDED/" >x <template &&
+       test_write_lines p 1 | git add -i >output &&
+       grep firstline output &&
+       test_write_lines c 0 p 1 | git add -i >output &&
+       ! grep firstline output
+'

This script does have its share of bare `grep` invocations, but these
days we prefer `test_grep`, which also appears often in this script,
so the following would be more appropriate:

    test_grep firstline output &&
    ...
    test_grep ! firstline output

Note the placement of "!" when used with `test_grep`.