git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] progress.c tests: fix breakage with COLUMNS != 80

From
Philip Oakley <philipoakley@iee.email>
Date
Jun 24, 2021, 15:05 UTC
Message-ID
<63a4aa3c-590d-f00f-2550-2564964f054a@iee.email>
In-Reply-To
<YNPISWEBxISC30DW@coredump.intra.peff.net>
On 24/06/2021 00:48, Jeff King wrote:
Show 32 quoted lines
> On Mon, Jun 21, 2021 at 09:01:23AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> The tests added in 2bb74b53a49 (Test the progress display, 2019-09-16)
>> broke under anything except COLUMNS=80, i.e. when running them under
>> the "-v" mode under a differently sized terminal.
>>
>> Let's set the expected number of COLUMNS at the start of the test to
>> fix that bug. It's handy not do do this in test-progress.c itself, in
>> case we'd like to test for a different number of COLUMNS, either
>> manually or in a future test.
> Hmm. So I can easily reproduce the problem here, and your patch fixes
> it. But my first thought was: shouldn't test-lib.sh be handling this to
> give all of the scripts a uniform environment?
>
> And indeed, we _do_ unset COLUMNS there. So I think the problem isn't
> a bad setting of $COLUMNS, but rather that in "-v" mode, the
> sub-command's stderr is hooked to our tty, and term_columns() is smart
> enough to use TIOCGWINSZ to get the value (at least on some platforms).
>
> Setting $COLUMNS again in the environment fixes it, because we prefer
> that value to trying the ioctl.
>
> So I don't think what you have here is wrong (though the commit message
> is a little misleading). But it seems like the original intent of our
> "unset COLUMNS" in test-lib.sh would best be fulfilled by setting it to
> a known value there (like 80), rather than unsetting it.
>
> I admit this a _bit_ of a nitpick (since as far as we know none of the
> other scripts care about the terminal width), so I'm OK with this as-is
> if you feel strongly the other way.
>
> -Peff

There has been a similar recent issue [1] on Git for Windows regarding these settings where different terminals had different views  about the size of the display window which resulted in incorrect out puts.

Philip
[1] https://github.com/git-for-windows/git/issues/3235
Previous: Jeff KingNext: Ævar Arnfjörð Bjarmason
Message 5 of 10 in “progress.c tests: fix breakage with COLUMNS != 80”
  1. progress.c tests: fix breakage with COLUMNS != 80Ævar Arnfjörð Bjarmason, Jun 21, 2021
  2. Jeff KingJun 23, 2021
  3. SZEDER GáborJun 24, 2021
  4. Jeff KingJun 24, 2021
  5. Philip OakleyJun 24, 2021
  6. test-lib.sh: set COLUMNS=80 for --verbose repeatabilityÆvar Arnfjörð Bjarmason, Jun 24, 2021
  7. Jeff KingJun 24, 2021
  8. SZEDER GáborJun 27, 2021
  9. Junio C HamanoJun 29, 2021
  10. test-lib.sh: set COLUMNS=80 for --verbose repeatabilityÆvar Arnfjörð Bjarmason, Jun 29, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.