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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html