Jeff King <peff@xxxxxxxx> writes:
>> 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.
>
>> 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.
