Re: [PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS

Jeff King [off-list ref] writes:

quoted

Let's instead solve this more thoroughly. We'll now take
GIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the
COLUMNS variable to break any tests that rely on it being set to a
sane value.

If something breaks because we have a codepath that's not
term_columns() checking COLUMNS we'd like to know about it, the narrow
"shopt -u checkwinsize" fix won't give us that.

I guess people running with bash won't see the test breakage (because
bash will quietly "fix" the COLUMNS setting). But enough people run with
/bin/sh that we'll eventually notice.

quoted

This approach does mean that any tests of ours that expected to test
term_columns() behavior by setting COLUMNS will need to explicitly
unset GIT_TEST_COLUMNS, or set it to the empty string. I'm doing the
latter in all the tests changed here.

This is rather ugly, and I'm not in general a fan of adding more
test-only code into the bowels of Git itself. But it may be the least
bad solution.

Yeah, this really look unsatisfactory, especially with this repeated
pattern that is hard to read:

+	GIT_TEST_COLUMNS= COLUMNS=81 git branch --column=column >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git branch --column=column >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git branch >actual &&
+	GIT_TEST_COLUMNS= COLUMNS=80 git branch -v | cut -c -8 | sed "s/ *$//" >actual &&

Perhaps with something like

	test_with_columns () {
        	local columns=$1
		shift
		GIT_TEST_COLUMNS= COLUMNS=$columns "$@"
	}

we may be able to hide the ugly implementation detail like this:

	test_with_columns 81 git branch --column=column >actual

and may become a bit more palatable?  A good thing is that this can
be done as two-step process, with its first step being 

    s/^(\s*)COLUMNS=(\d+)/$1test_with_columns $2/;

plus addition of the helper to test-lib, perhaps like so:

	test_with_columns () {
        	local columns=$1
		shift
		COLUMNS=$columns "$@"
	}

and the whole GIT_TEST_COLUMNS stuff being the second step.