Skip site navigation (1) Skip section navigation (2)

Re: Proposed patch - psql wraps at window width

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Gregory Stark <stark(at)enterprisedb(dot)com>
Cc: Brendan Jurd <direvus(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Bryce Nesbitt <bryce2(at)obviously(dot)com>, heikki(at)enterprisedb(dot)com
Subject: Re: Proposed patch - psql wraps at window width
Date: 2008-04-24 14:28:25
Message-ID: 200804241428.m3OESPD21700@momjian.us (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
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. +

In response to

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2008-04-24 14:42:11
Subject: Re: Avoiding a needless failure of PITR
Previous:From: Fujii MasaoDate: 2008-04-24 14:25:00
Subject: Avoiding a needless failure of PITR

pgsql-patches by date

Next:From: Alvaro HerreraDate: 2008-04-24 14:42:06
Subject: Re: lc_time and localized dates
Previous:From: Tom LaneDate: 2008-04-24 14:25:25
Subject: Re: Improve shutdown during online backup, take 4

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group