Re: [PATCH v7 5/8] merge: cleanup messages like commit
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Mar 11, 2019, 10:14 UTC
- Message-ID
- <2e10d78a-d92c-76c1-abc3-49ad1be55779@gmail.com>
- In-Reply-To
- <CAPig+cS5YsjxuLGNAtfFguQvy2p2bJECSsHksx356+WsO1ct5w@mail.gmail.com>
Hi Denton/Eric
On 11/03/2019 05:49, Eric Sunshine wrote:
Show 24 quoted lines
> On Sun, Mar 10, 2019 at 11:42 PM Denton Liu <liu.denton@gmail.com> wrote: >> This change allows git-merge messages to be cleaned up with the >> commit.cleanup configuration or --cleanup option, just like how >> git-commit does it. >> >> We also give git-pull the passthrough option of --cleanup so that it can >> also take advantage of this change. >> >> Finally, add testing to ensure that messages are properly cleaned up. >> Note that some newlines that were added to the commit message were >> removed so that if a file were read via -F, it would be copied >> faithfully. >> >> Signed-off-by: Denton Liu <liu.denton@gmail.com> >> --- >> diff --git a/t/t7604-merge-custom-message.sh b/t/t7604-merge-custom-message.sh >> @@ -47,4 +47,65 @@ test_expect_success 'merge --log appends to custom message' ' >> +test_expect_success 'cleanup commit messages (verbatim option)' ' >> + git reset --hard c1 && >> + git merge --cleanup=verbatim -F expect c2 && >> + git cat-file -p HEAD |sed -e "1,/^\$/d" >actual && > > An earlier patch in this series "fixed" a test with a Git command > upstream of a pipe. Yet, this test adds such an instance.
Is it worth worrying about? We're not trying to test cat-file here and if it does fail the test will fail as actual will be empty. You could use git log --pretty=%B >actual which is clearer and saves forking sed.
Best Wishes
Phillip
Show 85 quoted lines
> (Also, style: add space after '|'.)
>
>> + test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'cleanup commit messages (whitespace option)' '
>> + git reset --hard c1 &&
>> + { echo;echo "# text";echo; } >text &&
>
> Style: add space after semicolon or use &&-chaining inside {...}.
>
> Alternately, less ugly:
>
> test_write_lines "" "# text" "" >text &&
>
> (Or even a here-doc, though the leading and trailing blank lines make
> the here-doc ugly.)
>
>> + echo "# text" >expect &&
>> + git merge --cleanup=whitespace -F text c2 &&
>> + git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
>
> Git upstream pipe.
>
>> + test_cmp expect actual
>> +
>> +'
>
> Style: drop the blank line before the closing quote.
>
>> +test_expect_success 'cleanup merge messages (scissors option)' '
>> + git reset --hard c1 &&
>> + cat >text <<EOF &&
>
> This here-doc probably ought to be using '-' and '\', so:
>
> cat >text <<-\EOF &&
>
> and indent the here-doc body.
>
>> +# to be kept
>> +
>> + # ------------------------ >8 ------------------------
>> +# to be kept, too
>> +# ------------------------ >8 ------------------------
>> +to be removed
>> +# ------------------------ >8 ------------------------
>> +to be removed, too
>> +EOF
>> +
>> + cat >expect <<EOF &&
>
> Ditto: <<-\EOF and indent body.
>
>> +# to be kept
>> +
>> + # ------------------------ >8 ------------------------
>> +# to be kept, too
>> +EOF
>> + git merge --cleanup=scissors -e -F text c2 &&
>> + git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
>
> Git upstream pipe.
>
>> + test_cmp expect actual
>> +'
>> +
>> +test_expect_success 'cleanup commit messages (strip option)' '
>> + git reset --hard c1 &&
>> + { echo;echo "# text";echo sample;echo; } >text &&
>
> test_write_lines "" "# text" sample "" >text &&
>
>> + echo sample >expect &&
>> + git merge --cleanup=strip -F text c2 &&
>> + git cat-file -p HEAD |sed -e "1,/^\$/d">actual &&
>
> Git upstream pipe.
>
>> + test_cmp expect actual
>> +
>> +'
>
> Drop blank line before closing quote.
>