Re: Proposed patch - psql wraps at window width - Mailing list pgsql-hackers

Gregory Stark wrote:
> "Bruce Momjian" <bruce@momjian.us> writes:
> > Gregory Stark wrote:
> >> Earlier I suggested -- and nobody refuted -- that we should follow the
> >> precedents of ls and man and other tools which need to find the terminal
> >> width: Explicitly set width takes precedence always, if it's not explicitly
> >> set then you use the ioctl, and if that fails then you use the COLUMNS
> >> environment variable.
> >
> > Yes, I like that better.  Patch updated, same URL:
> >
> >     ftp://momjian.us/pub/postgresql/mypatches/wrap
> 
> I think it should just be:
> 
>     if (opt->format == PRINT_WRAP)
>     {
>         /* Get terminal width --  explicit setting takes precedence */
>         output_columns = opt->columns;
> 
> #ifdef TIOCGWINSZ
>         if (output_columns == 0 && isatty(fout))
>         {
>             struct winsize screen_size;
> 
>             if (ioctl(fileno(fout), TIOCGWINSZ, &screen_size) != -1)
>                 output_columns = screen_size.ws_col;
>         }
> #endif
> 
>         if (output_columns == 0)
>         {
>             const char *columns_env = getenv("COLUMNS");
> 
>             if (columns_env)
>                 output_columns = atoi(columns_env);
>         }
> 
>         if (output_columns == 0)
>             output_columns = 79;
>     }
> 
> 
> The differences this makes are that:
> 
> a) if you do -o /dev/tty (perhaps on some kind of cronjob) it will use the
>    ioctl.

Uh, if you do that I am not sure what the user would want.  I duplicated
what we do with PAGER and unless there is a clear mandate I think we
should keep the wrapping detection consistent with that;  we have gotten
no complaints.  Pager will not work for -o /dev/tty either.

> b) If you dump to a file it will still respect COLUMNS. This might be a bit
>    weird since bash sets COLUMNS so your file width will be based on the size
>    of your terminal. But people also do things like COLUMNS=120 psql -o f ...

No, that will make the regression tests fail and it is hard to say why
you would want a file wrap width to match your screen width.

Your final change is to assume a width of 79 if no columns are detected.
I also don't think that is a good idea, and if we want to do that we
would need to discuss that too.

I don't want to over-design this.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +