Re: [PATCH v4 4/4] add-patch: add diff.context command line overrides

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Leon

On 19/07/2025 13:28, Leon Michalak via GitGitGadget wrote:
From: Leon Michalak <leonmichalak6@xxxxxxxxx>

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.

-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.interHunkContext

+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 @@" actual

This is also deleting a test added in the last patch

+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.

+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
+	'
+done

This looks good as well
  test_done
As 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.

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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux