Re: [PATCH v3 1/2] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 20, 2011, 19:12 UTC
- Message-ID
- <7vaa9z6n9p.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1316506892-6072-1-git-send-email-zapped@mail.ru>
Alexey Shumkin <zapped@mail.ru> writes:
Show 5 quoted lines
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh > ... > @@ -48,28 +53,25 @@ head2=$(add_file sm1 foo3) > > test_expect_success 'modified submodule(forward)' "
As this is [PATCH 1/2], doesn't this patch make this test fail, calling for test_expect_failure here (and then later in 2/2 to be flipped back to test_expect_success)?
Show 10 quoted lines
> git diff-index -p --submodule=log HEAD >actual && > - cat >expected <<-EOF && > -Submodule sm1 $head1..$head2: > - > Add foo3 > -EOF > + printf \"Submodule sm1 $head1..$head2:\n\ > + > Add foo3 ($added foo3)\n\ > +\" > expected && > test_cmp expected actual > "
Hmmm... why printf? Is it worth to force reviewers wonder what would happen if any of these $variables happened to have "%" in them? Are you benefiting from any printf features that you cannot achieve with the original cat?
This hunk and others throughout your patch that change cat with here doc into printf seem to make the tests less legible, at least to me.
Perhaps like this instead, if the "flushed left" of the original looked ugly to your eyes?
@@ -49,27 +54,25 @@ head2=$(add_file sm1 foo3) test_expect_success 'modified submodule(forward)' " git diff-index -p --submodule=log HEAD >actual && cat >expected <<-EOF && -Submodule sm1 $head1..$head2: - > Add foo3 -EOF + Submodule sm1 $head1..$head2: + > Add foo3 ($added foo3) + EOF test_cmp expected actual "