Re: [PATCH 1/3] add-patch: respect diff.context configuration
From: Junio C Hamano <hidden>
Date: 2025-05-06 17:29:39
"Leon Michalak via GitGitGadget" [off-list ref] writes:
From: Leon Michalak <redacted> This aims to teach relevant builtins (that take in `--patch`) to respect the user's diff.context and diff.interHunkContext file configurations. Since these are both UI options and `--patch` is designed for the end user, I believe this was previously just an inconsistency, which this patch hopes to address.
The usual way to compose a log message of this project is to - Give an observation on how the current system works in the present tense (so no need to say "Currently X is Y", just "X is Y"), and discuss what you perceive as a problem in it. - Propose a solution (optional---often, problem description trivially leads to an obvious solution in reader's minds). - Give commands to the codebase to "become like so". in this order. "Various built-in commands that use add-patch infrastructure do not honor diff.context and diff.interHunkContext configuration variables." Would be the first sentence to explain the current situation. Follow that sentence with an explanation why it is a bad thing (i.e. it is possible that "do not honor" may be a deliberate design decision---perhaps the implementation of "add -p" is impossible if it has to honor diff.context that is set to 0; show evidence that no such deliberate design decision was there when the feature was introduced from the list archive, and say that not honoring the configuration is an inconvenient inconsistency). Such an explanation would be sufficient for readers to guess the "solution", so stopping there should be fine. Thanks.
quoted hunk
@@ -1014,10 +1019,13 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps, if (count > 0) { struct child_process cmd = CHILD_PROCESS_INIT; - strvec_pushl(&cmd.args, "git", "diff", "-p", "--cached", - oid_to_hex(!is_initial ? &oid : - s->r->hash_algo->empty_tree), - "--", NULL); + strvec_pushl(&cmd.args, "git", "diff", "-p", "--cached", NULL); + if (s->context != -1) + strvec_pushf(&cmd.args, "--unified=%i", s->context); + if (s->interhunkcontext != -1) + strvec_pushf(&cmd.args, "--inter-hunk-context=%i", s->interhunkcontext); + strvec_pushl(&cmd.args, oid_to_hex(!is_initial ? &oid : + s->r->hash_algo->empty_tree), "--", NULL);
OK.
+test_expect_success 'diff.context honored by "add"' ' + git add -p >output && + ! grep firstline output &&
test_grep ! firstline output ?
+ git config diff.context 8 && + git add -p >output && + grep "^ firstline" output &&
Likewise. test_grep "^firstline" output ?
+ git config --unset diff.context
Not like this. Either use test_when_finished to arrange that this unset is run when you leave, or use test_config. test_expect_success title ' test_config diff.context 8 && do your tests ' will reset diff.conftext when the above piece is done. test_expect_success title ' test_when_finished "git config unset diff.context" && git config set diff.context 8 && do your tests ' is an equivalent.