Re: [PATCH v3] branch: introduce --show-current display option

Daniels Umanovskis <daniels@umanovskis.se> writes:

Thanks for fixing this (I fixed this in the previous round in my tree but forgot to tell you about it).

OK, that's trivial. We checkout a branch and make sure show-current reports the name of that branch. Good.

OK, and at the same time we make sure the command exits with success. Good.

It is a bit curious why you remove the branch but not the tag after this test. If we are cleaning after ourselves, removing both would be equally good, if not cleaner. If having both absolutely harms later tests but having just one is OK, then any failure in this test between the time branch-and-tag-name tag gets created and the time branch-and-tag-name branch gets removed will leave the repository with both the tag and the branch, which will be the state in which later tests start, so having "branch -d" at this spot in the sequence is not a good idea anyway.

So two equally valid choices are to remove "branch -d" and then either:

 (1) leave both branch and tag after this test in the test
     repository

 (2) use test_when_finished, i.e.

	echo branch-and-tag-name >expect &&
	test_when_finished "git branch -D branch-and-tag-name" &&
	git checkout -b branch-and-tag-name &&
	test_when_finished "git tag -d branch-and-tag-name" &&
	git tag branch-and-tag-name &&
	...

     to arrange them to be cleaned once this test is done.

(1) is only valid if they do not harm later tests. I guess you remove the branch because you did not want to touch later tests that checks output from "git branch --list", in which case you'd want (2).

Please do *not* cd around without being in a subshell. If the second --show-current failed for some reason, "cd .." will not be executed, and the next and subsequent test will start inside ./worktree subdirectory, which is likely to break the expectations of them. Perhaps something like

	git checkout branch-one &&
	git worktree add worktree branch-two &&
	(
		git branch --show-current &&
		cd worktree && git branch --show-current
	) >actual &&
	test_cmp expect actual

or its modern equivalent

	git checkout branch-one &&
	git worktree add worktree branch-two &&
	(
		git branch --show-current &&
		git -C worktree branch --show-current
	) >actual &&
	test_cmp expect actual

Note that the latter _could_ be written without subshell, i.e.

	git branch --show-current >actual &&
	git -C worktree branch --show-current >>actual &&

but I personally tend to prefer with a single redirection into ">actual", as that is easier to later add _more_ commands to redirect into 'actual' to be inspected without having to worry about details like repeating ">>actual" or only the first one must be ">actual" (iow, the preference comes mostly from maintainability concerns).

Thanks.