Re: proposal (9.5) : psql unicode border line styles

Hi Tomas

2014-07-22 23:20 GMT+02:00 Tomas Vondra <tv(at)fuzzy(dot)cz>:

> On 28.6.2014 21:29, Pavel Stehule wrote:
> > Hello
> >
> > rebase for 9.5
> >
> > test:
> > \pset linestyle unicode \pset border 2
> > \pset unicode_header_linestyle double
> >
> > \l
> >
> > Regards
> >
> > Pavel
>
> I did a quick review of the patch today:
>
> * it applies cleanly to current HEAD (no failures, small offsets)
> * compiles and generally seems to work just fine
>
> Two questions:
>
> (1) Shouldn't the new options be listed in '\?' (as possible names for
> "pset")? I mean, here:
>
> \pset [NAME [VALUE]] set table output option
> (NAME :=
> {format|border|expanded|fieldsep|fieldsep_zero|footer|null|
> numericlocale|recordsep|recordsep_zero|tuples_only|title|tableattr|pager})
>
>
fixed

>
> (2) I noticed this piece of code:
>
> +typedef enum unicode_linestyle
> +{
> + UNICODE_LINESTYLE_SINGLE = 0, /* to make sure someone initializes
> this */
> + UNICODE_LINESTYLE_DOUBLE = 1
> +} unicode_linestyle;
>
> Why are the values defined explicitly? These values are set by the
> compiled automatically, so why set them manually? Only a few of the
> other enums are defined explicitly, and most of them have to do that to
> define different values (e.g. 0x01, 0x02, 0x04, ...).
>

this is useless - I removed it.

>
> I don't understand how the comment "to make sure someone initializes
> this" explains the purpose?
>

copy/paste error :( - removed

updated version is in attachment

Regards

Pavel

>
>
> regards
> Tomas
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>