On Thu, Nov 15, 2012 at 08:25:26AM -0800, Jeff King wrote:
> > Thanks, this version looks good to me.
>
> Oh wait. I did not look closely enough. The point was to move the option
> parser _out_ of git_diff_ui_config into git_diff_basic_config, so that
> it only triggers for porcelain, not plumbing.
Oh no, I am having a muddled morning. It _is_ right. We want to move it
out of basic into ui. Sorry for the noise. I just woke up and am
thinking backwards.
It may be worth squashing this test into patch 3:
diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
index e401814..023439f 100755
--- a/t/t4041-diff-submodule-option.sh
+++ b/t/t4041-diff-submodule-option.sh
@@ -72,6 +72,21 @@ EOF
test_cmp expected actual
"
+test_expect_success 'diff.submodule does not affect plumbing' '
+ test_config diff.submodule log &&
+ git diff-index -p HEAD >actual &&
+ cat >expected <<-EOF &&
+ diff --git a/sm1 b/sm1
+ new file mode 160000
+ index 0000000..a2c4dab
+ --- /dev/null
+ +++ b/sm1
+ @@ -0,0 +1 @@
+ +Subproject commit $fullhead1
+ EOF
+ test_cmp expected actual
+'
+
commit_file sm1 &&
head2=$(add_file sm1 foo3)
BTW, while writing the test, I noticed two minor nits with your tests:
1. They can use test_config, which is simpler (you do not need to
unset yourself after the test) and safer (the unset happens via
test_when_finished, so it works even if the test fails).
2. You can still indent expected output when using <<-.
I don't know if it is worth re-rolling for them.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html