Re: Proposed patch - psql wraps at window width

From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Bruce Momjian" <bruce(at)momjian(dot)us>
Cc: "Bryce Nesbitt" <bryce2(at)obviously(dot)com>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>, "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, "Brendan Jurd" <direvus(at)gmail(dot)com>, <heikki(at)enterprisedb(dot)com>
Subject: Re: Proposed patch - psql wraps at window width
Date: 2008-04-29 13:26:28
Message-ID: 87y76w4wsb.fsf@oxford.xeocode.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Bruce Momjian" <bruce(at)momjian(dot)us> writes:

> Gregory Stark wrote:
>> "Bruce Momjian" <bruce(at)momjian(dot)us> writes:
>>
>> > We do look at COLUMNS if the ioctl() fails, but not for file/pipe
>> > output.
>>
>> Yeah, it looks like your most recent patch still has the bug that if the user
>> specifies wrapped there are some complicated rules creating cases where it
>> will ignore the user's request and use un-wrapped output instead.
>
> Can you be more specific? You mean if the headings don't fit? Yea,
> that is true. I am thinking of adding a \pset auto format to \x in
> those cases, but that if for later.

[No I wasn't thinking of that, that's an interesting case too though I think
we might need to think a bit harder about cases that wrap poorly. If you have
long column headings we could wrap those too. But what if you have enough
space for just a few characters per column and you have long text fields in
those columns?]

I just meant the same thing I've been on about all week. Currently the
decision about whether to use wrapped mode is tied up with the decision on
what width to use and the result is that we ignore -Pformat=wrapped according
to some arcane set of rules.

The cases where we ignore the user's selected format are quite complex and not
accurately described in the documentation. They're also not accurately
described by your "not for file/pipe output" description either.

An accurate description would appear to be something like:

<quote>Wrapped</quote> is like <literal>aligned</> but wraps to a target
width of <literal>\pset columns</> or the width of the screen (unless screen
size determination fails or output has been redirected using -o or \o in
which case it is ignored and psql uses normal aligned mode unless \pset
columns is used).

It's confusing and inconsistent. I think it's better to pick a simple set of
general principles and code to that. Trying to code to presumed use cases
often ends up with code which handles corner cases poorly or inconsistently.

I think the behaviour should be simply:

format=auto
isatty(fout) ? format := wrapped : format := aligned
format=wrapped
columns := \pset columns || ioctl(fout) || getenv(COLUMNS) || 79

[Note in the above that the ioctl is on fout, not stdout!]

That would be easy to explain in the documentation as two simple consistent
rules. And as a bonus it would be consistent with other programs which use
these variables.

So the description I would code to is simply:

"Wrapped" is like aligned but wraps to \pset columns or an automatically
determined screen size. The screen size is determined automatically if output
is to a terminal which supports that, if that fails then by checking the
COLUMNS environment variable, and if that's unset then by defaulting to 79.

"Auto" selects "wrapped" format when output is a terminal and "aligned"
format otherwise.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's Slony Replication support!

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message PFC 2008-04-29 13:29:44 Re: Protection from SQL injection
Previous Message PontoSI - Consultoria, Informática e Serviços LDA 2008-04-29 13:19:24 table format specification