Re: review: psql and pset without any arguments

Поиск
Список
Период
Сортировка
От Gilles Darold
Тема Re: review: psql and pset without any arguments
Дата
Msg-id 5247E8D8.4050106@dalibo.com
обсуждение исходный текст
Ответ на Re: review: psql and pset without any arguments  (Pavel Stehule <pavel.stehule@gmail.com>)
Ответы Re: review: psql and pset without any arguments  (Pavel Stehule <pavel.stehule@gmail.com>)
Re: review: psql and pset without any arguments  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
Le 07/09/2013 21:22, Pavel Stehule a écrit :



2013/9/7 Gilles Darold <gilles.darold@dalibo.com>
Le 07/09/2013 10:02, Pavel Stehule a écrit :
> Hello
>
> * patch is cleanly patchable and compilation is without warnings
> * all regression tests passed
> * no impact on dump, performance or any current features
> * no comments to programming style
> * we would this feature - it is consistent with \set and it gives a
> fast resume about psql printer settings
>
> Issues:
> 1) it doesn't show linestyle when is default
>
> I looked there and it is probably a small different issue - this value
> is initialized as NULL as default. But I dislike a empty output in
> this case:
>
>     else if (strcmp(param, "linestyle") == 0)
>     {
>         if (!popt->topt.line_style)
>             ;
>         else
>             printf(_("Line style is %s.\n"),
>                    get_line_style(&popt->topt)->name);
>     }
>
> I propose a verbose result instead nothing:
>
>     else if (strcmp(param, "linestyle") == 0)
>     {
>         if (!popt->topt.line_style)
>            printf(_("Line style is unset.\n")) ;
>         else
>             printf(_("Line style is %s.\n"),
>                    get_line_style(&popt->topt)->name);
>     }

+1 to show the information even if linestyle is not set but by default
get_line_style() return "ascii" if linestyle is not set. So maybe it is
better to  rewrite it as follow:

    else if (strcmp(param, "linestyle") == 0)
    {
            printf(_("Line style is %s.\n"),
                   get_line_style(&popt->topt)->name);
    }

This will output:

    Line style is ascii.

when linestyle is not set or of course it is set to ascii.

> 2) there is only one open question
> http://www.postgresql.org/message-id/B6F6FD62F2624C4C9916AC0175D56D880CE00E0E@jenmbs01.ad.intershop.net
> - there is no clean relation between output and some pset option.
>
> I don't think so Marc' proposal is ideal (these values are not a
> variables) - but maybe some enhanced output (only for this resume) can
> be better:
>
> postgres=# \pset
> Output format (format) is aligned.
> Border style (border) is 1.
> Expanded display (expanded) is off.
> Null display (null) is "".
> Field separator (fieldsep) is "|".
> Tuples only (tuples_only) is off.
> Title (title) is unset.
> Table attributes (tableattr) unset.
> Pager (pager) is used for long output.
> Record separator (recordsep) is <newline>.
>
> This expanded output should be used only for this resume (not when a
> option was changed or individual ask on option value)

Yes this could be a good accommodation but I really prefer to not
duplicate code and translation between this resume and the output when
these options are set. If we can print the same output messages using:

    postgres=# \pset fieldsep '|'
    Field separator (fieldsep) is "|".

it could be a good compromise.

ok

Pavel

Hello,

Sorry for the delay, here is the new patch. The \pset output will look like follow:

postgres=# \pset
Border style (border) is 1.
Target width (columns) unset.
Expanded display (expanded) is off.
Field separator (fieldsep) is "|".
Default footer (footer) is on.
Output format (format) s aligned.
Line style (linestyle) is ascii.
Null display (null) is "".
Locale-adjusted numeric output (numericlocale) is off.
Pager (pager) is used for long output.
Record separator (recordsep) is <newline>.
Table attributes (tableattr) unset.
Title (title) unset.
Tuples only (tuples_only) is off.

postgres=# \pset null #
Null display (null) is "#".
postgres=#

This also mean that all translation strings of those messages should be updated.

If we don't want to modify those messages, I can provide an other patch which print output as follow:

postgres=# \pset
border: Border style is 1.
columns: Target width unset.
expanded: Expanded display is off.
fieldsep: Field separator is "|".
footer: Default footer is on.
format: Output format is aligned.
linestyle: Line style is ascii.
null: Null display is "".
numericlocale: Locale-adjusted numeric output is off.
pager: Pager is used for long output.
recordsep: Record separator is <newline>.
tableattr: Table attributes unset.
title: Title unset.
tuples_only: Tuples only is off.

postgres=# \pset null #
Null display is "#".
postgres=#
I think the first output is better but it need translation work. Let me know.

Regards,

-- 
Gilles Darold
Administrateur de bases de données
http://dalibo.com - http://dalibo.org
Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Ian Lawrence Barwick
Дата:
Сообщение: Patch: FORCE_NULL option for copy COPY in CSV mode
Следующее
От: David Rowley
Дата:
Сообщение: Re: appendStringInfo vs appendStringInfoString