Hi Phillip,

On Mon, 1 Jun 2020, Phillip Wood wrote:

On 29/05/2020 03:38, Johannes Schindelin wrote:

quoted

On Wed, 27 May 2020, Phillip Wood wrote:

quoted

From: Rohit Ashiwal <redacted>

[...]

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.

quoted

	git diff --exit-code main

quoted

+	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