Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> On Thu, 17 Jun 2021, Junio C Hamano wrote:
>
>> I think treating this as "less" specific band-aid is OK, but I do
>> not think tying this to Windows is a good design choice.
>>
>> The guiding principle for this change is more like "if we do not
>> know and cannot learn the true value, internally assuming 80-columns
>> as a last resort fallback may be OK, but do not export it for
>> consumption for other people---they cannot tell if COLUMNS=80 they
>> see us export is because we actually measured the terminal width and
>> know it to be 80, or we just punted and used a fallback default", I
>> think, and there is nothing Windows-specific in there, no?
>>
>> In other words, if we use something like the attached as a "less
>> specific band-aid" for now (i.e. direct replacement of your patch to
>> fix the specific 'less' problem), and then later clean it up by
>> actually returning -1 (or -80) from term_columns() as "we do not
>> know" (or "we do not know---use the negation of this value as
>> default"), we can help not just this paticular caller you touched,
>> but all other callers of term_columns(), to make a more intelligent
>> decision in the future if they wanted to. The root of the issue I
>> think is because term_columns() does not give callers to tell if its
>> returned value is merely a guess.
>
> That approach should also work. Do you want me to take custody of your
> patch and issue a v3? If yes, I will mark you as co-author because the
> patch is not really only mine any longer.
Surely. The fewer conditionally compiled codepaths based on
platforms we have, the easier it becomes to reason about the code, I
would think.
>> pager.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git c/pager.c w/pager.c
>> index 3d37dd7ada..52f27a6765 100644
>> --- c/pager.c
>> +++ w/pager.c
>> @@ -11,6 +11,10 @@
>> static struct child_process pager_process = CHILD_PROCESS_INIT;
>> static const char *pager_program;
>>
>> +/* Is the value coming back from term_columns() just a guess? */
>> +static int term_columns_guessed;
>> +
>> +
>> static void close_pager_fds(void)
>> {
>> /* signal EOF to pager */
>> @@ -114,7 +118,8 @@ void setup_pager(void)
>> {
>> char buf[64];
>> xsnprintf(buf, sizeof(buf), "%d", term_columns());
>> - setenv("COLUMNS", buf, 0);
>> + if (!term_columns_guessed)
>> + setenv("COLUMNS", buf, 0);
>> }
>>
>> setenv("GIT_PAGER_IN_USE", "true", 1);
>> @@ -158,15 +163,20 @@ int term_columns(void)
>> return term_columns_at_startup;
>>
>> term_columns_at_startup = 80;
>> + term_columns_guessed = 1;
>>
>> col_string = getenv("COLUMNS");
>> - if (col_string && (n_cols = atoi(col_string)) > 0)
>> + if (col_string && (n_cols = atoi(col_string)) > 0) {
>> term_columns_at_startup = n_cols;
>> + term_columns_guessed = 0;
>> + }
>> #ifdef TIOCGWINSZ
>> else {
>> struct winsize ws;
>> - if (!ioctl(1, TIOCGWINSZ, &ws) && ws.ws_col)
>> + if (!ioctl(1, TIOCGWINSZ, &ws) && ws.ws_col) {
>> term_columns_at_startup = ws.ws_col;
>> + term_columns_guessed = 0;
>> + }
>> }
>> #endif
>>
>>
