Re: [PATCH] pager: set COLUMNS to term_columns()

From: Duy Nguyen <pclouds@gmail.com>
Date: May 11, 2018, 12:16 UTC
Message-ID: <CACsJy8CdnDYTd5mYe0R5z9cYRcezui-ViVSnvxO4xQLEWi_T5g@mail.gmail.com>
In-Reply-To: <20180511092515.GA9567@sigill.intra.peff.net>

On Fri, May 11, 2018 at 11:25 AM, Jeff King <peff@peff.net> wrote:

Show 17 quoted lines

> --- a/pager.c
> +++ b/pager.c
> @@ -109,10 +109,15 @@ void setup_pager(void)
>                 return;
>
>         /*
> -        * force computing the width of the terminal before we redirect
> -        * the standard output to the pager.
> +        * After we redirect standard output, we won't be able to use an ioctl
> +        * to get the terminal size. Let's grab it now, and then set $COLUMNS
> +        * to communicate it to any sub-processes.
>          */
> -       (void) term_columns();
> +       {
> +               char buf[64];
> +               xsnprintf(buf, sizeof(buf), "%d", term_columns());
> +               setenv("COLUMNS", buf, 0);

I wonder if this affects bash being a subprocess. E.g. if COLUMNS is exported will it still dynamically adjust COLUMNS? A quick test shows that it adjusts $COLUMNS anyway, so even if we need to launch a shell we should be good.

Show 7 quoted lines

> +       }
>
>         setenv("GIT_PAGER_IN_USE", "true", 1);
>
> --
> 2.17.0.984.g9b00a423a4
>

-- 
Duy