proposal (9.5) : psql unicode border line styles

Hello

I had to reduce allowed line style to single or double, because unicode
allows only combination single,double or single,thick

postgres=# \l
List of databases
Name | Owner | Encoding | Collate | Ctype | Access
privileges
-----------+----------+----------+-------------+-------------+-----------------------
postgres | postgres | UTF8 | en_US.UTF-8 | en_US.UTF-8 |
template0 | postgres | UTF8 | en_US.UTF-8 | en_US.UTF-8 |
=c/postgres +
| | | | |
postgres=CTc/postgres
template1 | postgres | UTF8 | en_US.UTF-8 | en_US.UTF-8 |
=c/postgres +
| | | | |
postgres=CTc/postgres
(3 rows)

postgres=# \pset border 2
Border style (border) is 2.
postgres=# \pset linestyle unicode
Line style (linestyle) is unicode.
postgres=# \l
List of databases
┌───────────┬──────────┬──────────┬─────────────┬─────────────┬───────────────────────┐
│ Name │ Owner │ Encoding │ Collate │ Ctype │ Access
privileges │
├───────────┼──────────┼──────────┼─────────────┼─────────────┼───────────────────────┤
│ postgres │ postgres │ UTF8 │ en_US.UTF-8 │ en_US.UTF-8
│ │
│ template0 │ postgres │ UTF8 │ en_US.UTF-8 │ en_US.UTF-8 │
=c/postgres ↵│
│ │ │ │ │ │
postgres=CTc/postgres │
│ template1 │ postgres │ UTF8 │ en_US.UTF-8 │ en_US.UTF-8 │
=c/postgres ↵│
│ │ │ │ │ │
postgres=CTc/postgres │
└───────────┴──────────┴──────────┴─────────────┴─────────────┴───────────────────────┘
(3 rows)

postgres=# \pset unicode_header_linestyle double
Unicode border linestyle is "double".
postgres=# \l
List of databases
┌───────────┬──────────┬──────────┬─────────────┬─────────────┬───────────────────────┐
│ Name │ Owner │ Encoding │ Collate │ Ctype │ Access
privileges │
╞═══════════╪══════════╪══════════╪═════════════╪═════════════╪═══════════════════════╡
│ postgres │ postgres │ UTF8 │ en_US.UTF-8 │ en_US.UTF-8
│ │
│ template0 │ postgres │ UTF8 │ en_US.UTF-8 │ en_US.UTF-8 │
=c/postgres ↵│
│ │ │ │ │ │
postgres=CTc/postgres │
│ template1 │ postgres │ UTF8 │ en_US.UTF-8 │ en_US.UTF-8 │
=c/postgres ↵│
│ │ │ │ │ │
postgres=CTc/postgres │
└───────────┴──────────┴──────────┴─────────────┴─────────────┴───────────────────────┘
(3 rows)

postgres=#

Regards

Pavel

2014-03-07 19:24 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

Hello

rebase for 9.5

test:
\pset linestyle unicode \pset border 2
\pset unicode_header_linestyle double

\l

Regards

Pavel

2014-03-11 21:17 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:

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})

(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, ...).

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

regards
Tomas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi Tomas

2014-07-22 23:20 GMT+02:00 Tomas Vondra <tv@fuzzy.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

On 23 Červenec 2014, 7:36, Pavel Stehule wrote:

updated version is in attachment

OK, thanks. The new version seems OK to me.

Tomas

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

2014-07-23 8:38 GMT+02:00 Tomas Vondra <tv@fuzzy.cz>:

On 23 Červenec 2014, 7:36, Pavel Stehule wrote:

updated version is in attachment

OK, thanks. The new version seems OK to me.

Thank you

Pavel

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

2014-07-23 8:38 GMT+02:00 Tomas Vondra <tv@fuzzy.cz>:

OK, thanks. The new version seems OK to me.

Thank you

I've started looking over the patch and went back through the previous
thread about it. For my part, I'm in favor of adding this capability,
but I'm not terribly happy about how it was done. In particular,
get_line_style() seems pretty badly hacked around, and I don't really
like having the prepare_unicode_format call underneath it allocating
memory and then passing back up the need to free that memory via a new
field in the structure. Also, on a quick glance, are you sure that the
new 'unicode' output matches the same as the old 'unicode' did (with
pg_utf8format)?

I would think we'd simply set up a structure which is updated when the
linestyle is changed, which is surely going to be much less frequently
than the request for which linestyle to use happens, and handle all of
the line styles in more-or-less the same way rather than doing something
completely different for unicode than for the others.

Thanks,

Stephen

Hi

I removed dynamic allocation and reduced patch size.

What I tested a old unicode style is same as new unicode style. There
nothing was changed .. some fields are specified in refresh_utf8format
function

Regards

Pavel

2014-09-08 4:44 GMT+02:00 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I removed dynamic allocation and reduced patch size.

This is certainly better, imv, though there are a couple of minor
issues (extra semi-colons, extraneous whitespace, get_line_style was
still changed to non-const, even though it doesn't need to be now).

What I tested a old unicode style is same as new unicode style. There
nothing was changed .. some fields are specified in refresh_utf8format
function

I don't particularly like this (having these fields set in
refresh_utf8format to hard-coded strings in the function), why not have
those handled the same as the rest, where the strings themselves are in
the unicode_style structure?

The rest looks pretty good. Need to step out for a bit but I'll look at
making the above changes when I get back if I don't hear anything.

Thanks,

Stephen

Hi

2014-09-11 16:42 GMT+02:00 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I removed dynamic allocation and reduced patch size.

This is certainly better, imv, though there are a couple of minor
issues (extra semi-colons, extraneous whitespace, get_line_style was
still changed to non-const, even though it doesn't need to be now).

fixed non-const -- other, I am sorry, I am blind

What I tested a old unicode style is same as new unicode style. There
nothing was changed .. some fields are specified in refresh_utf8format
function

I don't particularly like this (having these fields set in
refresh_utf8format to hard-coded strings in the function), why not have
those handled the same as the rest, where the strings themselves are in
the unicode_style structure?

I am not sure if I understand well.

With refresh_utf8format I can do shortly 6 possible combinations - or more
(when it will be requested)

I have no idea how to write as rest without repeating all 6 combinations -
what was one noticed issue of some older variant, where I designed
unicode1, unicode2, ...

Any idea, tip how to it?

Regards

Pavel

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

2014-09-11 16:42 GMT+02:00 Stephen Frost <sfrost@snowman.net>:

I don't particularly like this (having these fields set in
refresh_utf8format to hard-coded strings in the function), why not have
those handled the same as the rest, where the strings themselves are in
the unicode_style structure?

I am not sure if I understand well.

With refresh_utf8format I can do shortly 6 possible combinations - or more
(when it will be requested)

I have no idea how to write as rest without repeating all 6 combinations -
what was one noticed issue of some older variant, where I designed
unicode1, unicode2, ...

Any idea, tip how to it?

All I was suggesting was pulling these strings out of the function:

+   /* ↵ */
+   popt->header_nl_right = "\342\206\265";
+
+   popt->nl_left = " ";
+
+   /* ↵ */
+   popt->nl_right = "\342\206\265";
+
+   /* … */
+   popt->wrap_left = "\342\200\246";
+   popt->wrap_right = "\342\200\246";

and adding them to unicode_style and then referencing them there,
similar to how the rest of printTextFormat popt (by the way- don't
really like that variable name, particularly as it's used elsewhere with
a very different meaning.. why not 'format' or 'ptf'?) is
set up in refresh_utf8format, that's all.

Thanks,

Stephen

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

Any idea, tip how to it?

Attached is what I had been thinking.

Thoughts?

Thanks!

Stephen

2014-09-12 5:14 GMT+02:00 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

Any idea, tip how to it?

Attached is what I had been thinking.

Thoughts?

yes, it is better. I didn't understand before.

I though about it because it allows to design multiline end style in future.

I am not a fan of "↵" and "…" in resulted tables, and can be nice if I can
to optionally change it.

With your change we can design other "pure" style against current "rich"
style. But I don't have a idea, how to name it.

Regards

Pavel

2014-09-12 8:19 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

2014-09-12 5:14 GMT+02:00 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

Any idea, tip how to it?

Attached is what I had been thinking.

Thoughts?

yes, it is better. I didn't understand before.

I though about it because it allows to design multiline end style in
future.

I am not a fan of "↵" and "…" in resulted tables, and can be nice if I can
to optionally change it.

I though about it, and we cannot to drop it now. These symbols are
necessary, because we don't support line between rows.

I am thinking so 05 patch should be final

Regards

Pavel

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I though about it, and we cannot to drop it now. These symbols are
necessary, because we don't support line between rows.

I am thinking so 05 patch should be final

Ok. I'm going to play with it a bit more but barring issues, should get
it committed today.

Thanks!

Stephen

* Stephen Frost (sfrost@snowman.net) wrote:

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I though about it, and we cannot to drop it now. These symbols are
necessary, because we don't support line between rows.

I am thinking so 05 patch should be final

Ok. I'm going to play with it a bit more but barring issues, should get
it committed today.

Alright, pushed with some additional cleanup, and also cleaned up the
border documentation a bit.

Thanks!

Stephen

2014-09-12 18:09 GMT+02:00 Stephen Frost <sfrost@snowman.net>:

* Stephen Frost (sfrost@snowman.net) wrote:

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I though about it, and we cannot to drop it now. These symbols are
necessary, because we don't support line between rows.

I am thinking so 05 patch should be final

Ok. I'm going to play with it a bit more but barring issues, should get
it committed today.

Alright, pushed with some additional cleanup, and also cleaned up the
border documentation a bit.

Thank you very much

Regards

Pavel

Hi Stephen

I forgot to fix new enhanced pset help

fix attached

Regards

Pavel

2014-09-12 18:15 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:

On 2014-09-18 12:47:25 +0200, Pavel Stehule wrote:

-	output = PageOutput(81, pager);
+	output = PageOutput(87, pager);

fprintf(output, _("List of specially treated variables.\n"));

@@ -364,6 +364,10 @@ helpVariables(unsigned short int pager)
"                     column width of left aligned data type in latex format\n"));
fprintf(output, _("  title              set the table title for any subsequently printed tables\n"));
fprintf(output, _("  tuples_only        if set, only actual table data is shown\n"));
+	fprintf(output, _("  unicode_border_linestyle\n"));
+	fprintf(output, _("  unicode_column_linestyle\n"));
+	fprintf(output, _("  unicode_header_linestyle\n"
+					 "                     set the style of unicode line drawing [single, double]\n"));

fprintf(output, _("\nEnvironment variables:\n"));
fprintf(output, _("Usage:\n"));

Either the current line count is wrong, or you added the wrong number of
new lines to PageOutput(). Your patch only adds four \n, while you
increased from 81 to 87.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers