Re: [PATCH v4 4/4] add-patch: add diff.context command line overrides
From: Leon Michalak <hidden>
Date: 2025-07-22 18:02:48
Hey Philip, looking again for a second time and re-reading previous replies on this thread, I think I misunderstood a previous reply. I will add these back as soon as I can get back to the PC, thanks for spotting that! On Tue, 22 Jul 2025 at 17:02, Phillip Wood [off-list ref] wrote:
Hi Leon On 19/07/2025 13:28, Leon Michalak via GitGitGadget wrote:quoted
From: Leon Michalak <redacted> This patch compliments the previous commit, where builtins that use add-patch infrastructure now respect diff.context and diff.interHunkContext file configurations. In particular, this patch helps users who don't want to set persistent context configurations or just want a way to override them on a one-time basis, by allowing the relevant builtins to accept corresponding command line options that override the file configurations. This mimics commands such as diff and log, which allow for both context file configuration and command line overrides.Thanks for updating the quoting in the tests. Unfortunately this patch now deletes the tests added in the last commit which I don't think is correct.quoted
-test_expect_success 'add -p respects diff.context' ' - test_write_lines a b c d e f g h i j k l m >file &&I think there is some confusion here - why are we deleting the tests added in the last commit? This removes the test coverage for diff.context and diff.interHunkContextquoted
+for cmd in add checkout restore 'commit -m file' +do + test_expect_success "${cmd%% *} accepts -U and --inter-hunk-context" ' + test_write_lines a b c d e f g h i j k l m n o p q r s t u v >file && + git add file && + test_write_lines a b c d e F g h i j k l m n o p Q r s t u v >file && + echo y | git -c diff.context=5 -c diff.interhunkcontext=1 \ + $cmd -p -U 4 --inter-hunk-context 2 >actual && + test_grep "@@ -2,20 +2,20 @@" actual + ' +done + +test_expect_success 'reset accepts -U and --inter-hunk-context' ' + test_write_lines a b c d e f g h i j k l m n o p q r s t u v >file && + git commit -m file file && + test_write_lines a b c d e F g h i j k l m n o p Q r s t u v >file && git add file && - test_write_lines a b c d e f G h i j k l m >file && - echo y | git -c diff.context=5 add -p >actual && - test_grep "@@ -2,11 +2,11 @@" actual + echo y | git -c diff.context=5 -c diff.interhunkcontext=1 \ + reset -p -U 4 --inter-hunk-context 2 >actual && + test_grep "@@ -2,20 +2,20 @@" actual ' -test_expect_success 'add -p respects diff.interHunkContext' ' - test_write_lines a b c d e f g h i j k l m n o p q r s >file && - git add file && - test_write_lines a b c d E f g i i j k l m N o p q r s >file && - echo y | git -c diff.interhunkcontext=2 add -p >actual && - test_grep "@@ -2,16 +2,16 @@" actualThis is also deleting a test added in the last patchquoted
+test_expect_success 'stash accepts -U and --inter-hunk-context' ' + test_write_lines a b c d e F g h i j k l m n o p Q r s t u v >file && + git commit -m file file && + test_write_lines a b c d e f g h i j k l m n o p q r s t u v >file && + echo y | git -c diff.context=5 -c diff.interhunkcontext=1 \ + stash -p -U 4 --inter-hunk-context 2 >actual && + test_grep "@@ -2,20 +2,20 @@" actual ' -test_expect_success 'add -p rejects negative diff.context' ' - test_config diff.context -1 && - test_must_fail git add -p 2>output && - test_grep "diff.context cannot be negative" output -'and so is this. The tests you're adding look good but we shouldn't be deleting the existing ones.quoted
+for cmd in add checkout commit reset restore "stash save" "stash push" +do + test_expect_success "$cmd rejects invalid context options" ' + test_must_fail git $cmd -p -U -3 2>actual && + cat actual | echo && + test_grep -e ".--unified. cannot be negative" actual && + + test_must_fail git $cmd -p --inter-hunk-context -3 2>actual && + test_grep -e ".--inter-hunk-context. cannot be negative" actual && + + test_must_fail git $cmd -U 7 2>actual && + test_grep -E ".--unified. requires .(--interactive/)?--patch." actual && + + test_must_fail git $cmd --inter-hunk-context 2 2>actual && + test_grep -E ".--inter-hunk-context. requires .(--interactive/)?--patch." actual + ' +doneThis looks good as wellquoted
test_doneAs I said last time I do not think the tests below add any value. They also do not compensate for the removal of the tests for diff.context that are deleted above as they all pass -U on the commandline.quoted
diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh index 1384a8195705..0158fe6568cb 100755 --- a/t/t4055-diff-context.sh +++ b/t/t4055-diff-context.sh@@ -58,6 +58,36 @@ test_expect_success 'The -U option overrides diff.context' ' test_grep ! "^ firstline" output ' +test_expect_success 'The -U option overrides diff.context for "add"' ' + test_config diff.context 8 && + git add -U4 -p >output && + test_grep ! "^ firstline" output +' + +test_expect_success 'The -U option overrides diff.context for "commit"' ' + test_config diff.context 8 && + ! git commit -U4 -p >output && + test_grep ! "^ firstline" output +' + +test_expect_success 'The -U option overrides diff.context for "checkout"' ' + test_config diff.context 8 && + git checkout -U4 -p >output && + test_grep ! "^ firstline" output +' + +test_expect_success 'The -U option overrides diff.context for "stash"' ' + test_config diff.context 8 && + ! git stash -U4 -p >output && + test_grep ! "^ firstline" output +' + +test_expect_success 'The -U option overrides diff.context for "restore"' ' + test_config diff.context 8 && + git restore -U4 -p >output && + test_grep ! "^ firstline" output +' + test_expect_success 'diff.context honored by "diff"' ' test_config diff.context 8 && git diff >output &&Thanks Phillip