On Thu, Jun 21, 2018 at 1:04 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Elijah Newren <newren@xxxxxxxxx> writes:
>
>> + git checkout B &&
>> + # This is indented with HT SP HT.
>> + echo " foo();" >>foo &&
>
> I often wonder, whenever I see a need for a comment like this, if
> saying the same thing in code to make it visible is cleaner and less
> error prone way to do so, i.e. e.g.
>
> echo "_ _foo();" | tr "_" "\011" >>foo &&
I can make that change. Should I make the same change to
t4015-diff-whitespace.sh, where I copied that comment and line of code
from? (In a different topic submission, of course.)
>> +# Rebase has lots of useful options like --whitepsace=fix, which are
>> +# actually all built in terms of flags to git-am. Since neither
>> +# --merge nor --interactive (nor any options that imply those two) use
>> +# git-am, using them together will result in flags like --whitespace=fix
>> +# being ignored. Make sure rebase warns the user and aborts instead.
>> +#
>> +
>> +test_run_rebase () {
>> + opt=$1
>> + shift
>> + test_expect_failure "$opt incompatible with --merge" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --merge A
>> + "
>> +
>> + test_expect_failure "$opt incompatible with --strategy=ours" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --strategy=ours A
>> + "
>> +
>> + test_expect_failure "$opt incompatible with --strategy-option=ours" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --strategy=ours A
>> + "
>> +
>> + test_expect_failure "$opt incompatible with --interactive" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --interactive A
>> + "
>> +
>> + test_expect_failure "$opt incompatible with --exec" "
>> + git checkout B^0 &&
>> + test_must_fail git rebase $opt --exec 'true' A
>> + "
>> +
>> +}
>> +
>> +test_run_rebase --whitespace=fix
>> +test_run_rebase --ignore-whitespace
>> +test_run_rebase --committer-date-is-author-date
>> +test_run_rebase -C4
>
> I happen to be from old school and "rebase" primarily means
> "format-patch piped to am" in my mind, so from that point of view,
> "test_run_rebase --OPT" that says "--OPT which is a valid option for
> the primary operating mode of rebase does not work with the other
> exotic modes of the command" is not all that bad, but I do not think
> that worldview holds for many people in general. Perhaps calling it
> something like "test_rebase_am_only" makes the intent clearer?
Well, if that's what rebase means in your mind, then I'm sure you'll
have fun commenting on my follow-on series. :-)
Anyway, sure, I'm happy to change the function name.
