Hi Junio,
On Tue, Sep 03, 2019 at 02:11:22PM -0700, Junio C Hamano wrote:
> A few test scripts assign a single LF to $LF, but that is already
> given by test-lib.sh to everybody.
I didn't know that 't/test-lib.sh' provided '$LF' (as I'm sure was the
case for the respective authors of those tests below ;-)), so removing
the bespoke definitions and relying on the one already defined makes
good sense.
> Remove the unnecessary reassignment.
I did find a couple of extra spots when I went looking through 't'
myself. I ran:
$ git grep "='$" -- t
which in addition to the three spots below turned up a couple more
interesting locations, as well as a few false positives. They are:
- t/t3005: this script calls the variable '$new_line', but could be
renamed to LF and then removed in a second patch
- t/valgrind/analyze.sh: this script also calls the variable
'$new_line', but does not ever source test-lib.sh, so I think this
spot can be ignored.
So I think that my recommendation is to either:
* introduce a prepratory patch that updates t3005 to rename
'$new_line' to '$LF', and then amend the patch below to also blow
away t3005's use of '$LF', or
* do both steps in one larger patch
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> t/t3404-rebase-interactive.sh | 2 --
> t/t4013-diff-various.sh | 3 ---
> t/t5515-fetch-merge-logic.sh | 3 ---
> 3 files changed, 8 deletions(-)
>
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 461dd539ff..31114d0bbb 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -155,8 +155,6 @@ test_expect_success 'rebase -x with empty command fails' '
> test_i18ncmp expected actual
> '
>
> -LF='
> -'
> test_expect_success 'rebase -x with newline in command fails' '
> test_when_finished "git rebase --abort ||:" &&
> test_must_fail env git rebase -x "a${LF}b" @ 2>actual &&
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index a9054d2db1..5ac94b390d 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -7,9 +7,6 @@ test_description='Various diff formatting options'
>
> . ./test-lib.sh
>
> -LF='
> -'
> -
> test_expect_success setup '
>
> GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
> diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
> index e55d8474ef..961eb35c99 100755
> --- a/t/t5515-fetch-merge-logic.sh
> +++ b/t/t5515-fetch-merge-logic.sh
> @@ -12,9 +12,6 @@ GIT_TEST_PROTOCOL_VERSION=
>
> . ./test-lib.sh
>
> -LF='
> -'
> -
> test_expect_success setup '
> GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
> GIT_COMMITTER_DATE="2006-06-26 00:00:00 +0000" &&
The rest of the patch here looks obviously good.
Thanks,
Taylor
