Hi Phillip, On Mon, 1 Jun 2020, Phillip Wood wrote: > On 29/05/2020 03:38, Johannes Schindelin wrote: > > > > On Wed, 27 May 2020, Phillip Wood wrote: > > > >> From: Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> > >> > >> [...] > >> diff --git a/t/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh > >> new file mode 100755 > >> index 0000000000..fb5e747e86 > >> --- /dev/null > >> +++ b/t/t3436-rebase-more-options.sh > >> @@ -0,0 +1,86 @@ > >> +#!/bin/sh > >> +# > >> +# Copyright (c) 2019 Rohit Ashiwal > >> +# > >> + > >> +test_description='tests to ensure compatibility between am and interactive backends' > >> + > >> +. ./test-lib.sh > >> + > >> +. "$TEST_DIRECTORY"/lib-rebase.sh > >> + > >> +# This is a special case in which both am and interactive backends > >> +# provide the same output. It was done intentionally because > >> +# both the backends fall short of optimal behaviour. > >> +test_expect_success 'setup' ' > >> + git checkout -b topic && > >> + q_to_tab >file <<-\EOF && > >> + line 1 > >> + Qline 2 > >> + line 3 > >> + EOF > >> + git add file && > >> + git commit -m "add file" && > >> + cat >file <<-\EOF && > >> + line 1 > >> + new line 2 > >> + line 3 > >> + EOF > >> + git commit -am "update file" && > >> + git tag side && > >> + > >> + git checkout --orphan master && > >> + sed -e "s/^|//" >file <<-\EOF && > >> + |line 1 > >> + | line 2 > >> + |line 3 > >> + EOF > >> + git add file && > >> + git commit -m "add file" && > >> + git tag main > >> +' > > > > The file contents are repeated in an awfully repetitive manner. That not > > only makes things a bit hard to read, it also makes it all too easy to > > slip in bugs by mistake. How about something like this instead? > > > > test_commit file && > > > > test_write_lines line1 Qline2 line3 >templ && > > > > q_to_tab <templ >file.t && > > git commit -m tab file.t && > > > > sed "s/Q/new /" <templ >file.t && > > git commit -m new file.t && > > git tag side && > > > > git checkout file -- && > > sed "s/Q/ /" <templ >file.t && > > git commit -m spaces file.t > > I'm not totally convinced by this, I'd prefer to be able to read the > contents rather than having to work out what sed is doing. What about > > test_write_lines "line 1" " line 2" "line 3" >file && > add and commit > test_write_lines "line 1" "new line 2" "line 3" >file && > commit and tag > test_write_lines "line 1" " line 2" "line 3" >file && > commit and tag > > It does not get rid of the repetition but it does dispense with having > the work out what sed and q_to_tab are doing Sure. Your version is still tons easier to parse for a human being. > > git diff --exit-code main > > > >> + test_must_fail git rebase --merge main side && > >> + git rebase --abort && > >> + git rebase --merge --ignore-whitespace main side && > >> + test_cmp expect file > >> +' > >> + > >> +test_expect_success '--ignore-whitespace is remembered when continuing' ' > >> + cat >expect <<-\EOF && > >> + line 1 > >> + new line 2 > >> + line 3 > >> + EOF > >> + ( > >> + set_fake_editor && > >> + FAKE_LINES="break 1" git rebase -i --ignore-whitespace main side > >> + ) && > >> + git rebase --continue && > >> + test_cmp expect file > > > > It is a bit funny to see these two invocations _specifically_ pulled out > > from the subshell, that's not how we do things in other test scripts: > > instead, we run all the Git commands _inside_ the subshell, and all the > > verifications after the subshell. > > The idea was to only set the variable where it is used. I understand that, but I think that it would be better to follow the existing pattern rather than introducing an inconsistent second one. Thanks, Dscho
