Re: Proposed patch - psql wraps at window width

Gregory Stark wrote:
> "Bruce Momjian" <bruce(at)momjian(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +