Unicode UTF-8 table formatting for psql text output
psql currently uses the ASCII characters '-' '|' and '+' to render
tables. However, most modern terminal emulators are capable of
displaying characters other than ASCII, including box drawing
characters which can be used to create rather more pleasing and
readable tables than ASCII punctuation can achieve.
The following set of patches firstly teach psql how to draw nice
tables, by abstracting the characters used for formatting tables into
a simple structure. The table formatting can then be passed to
functions needing to know how to draw tables, and they can then use
the provided symbols rather than hard-coding ASCII punctuation.
Tables for ASCII and Unicode UTF-8 are provided.
The conversion of print_aligned_text is a very straightforward
substitution. print_aligned_vertical is slightly more complex because
its use of string offsets to overwrite the record number assume
"1 byte = 1 character", which is no longer valid for multibyte
encodings such as UTF-8. This has been refactored to split out the
line drawing out into an inline helper function
(print_aligned_vertical_line) which actually makes the code more like
print_aligned_text with its _print_horizontal_line helper, and
contains no assumptions about indexes into character arrays.
Examples of this in action are shown at the end of this mail.
Portability: UTF-8 strings are encoded as standard C octal escapes,
and so are portable. I added UTF-8 comments to show what the numbers
encode, which can be removed if needed. The code depends on
nl_langinfo(3) from <langinfo.h>, but this is #ifdef'd to allow
building on systems without support. By default, an ASCII table is
used, which will result in identical behaviour to the current psql.
However, if nl_langinfo is available, and it reports that the locale
codeset is UTF-8, it will switch to using Unicode UTF-8 box-drawing
characters, which draw identical tables to the current psql, just with
different characters.
Extensibility: The table formatting can potentially be used to support
other character sets containing other box drawing characters, for
example IBM CP 437 or 850. However, I have just stuck with UTF-8 for
now!
To follow:
[PATCH 1/6] psql: Abstract table formatting characters used for different line types.
[PATCH 2/6] psql: Add table formats for ASCII and UTF-8
[PATCH 3/6] psql: Create table format
[PATCH 4/6] psql: Pass table formatting object to text output functions
[PATCH 5/6] psql: print_aligned_text uses table formatting
[PATCH 6/6] psql: print_aligned_vertical uses table formatting
In the examples below, I think there's just one minor issue, which is
a leading '-' with border=0 and expanded=1, which I just noticed while
sending this mail. I'll tidy that up and send another patch.
This is something I really think makes psql more readable and more
usable, which I've been working on over the last couple of nights, and
so here it is for your comments and criticism. I hope you find it
useful!
Regards,
Roger
Examples:
rleigh=# \pset border 0
Border style is 0.
rleigh=# SELECT * FROM package_priorities;
id name
── ─────────
1 extra
2 important
3 optional
4 required
5 standard
(5 rows)
rleigh=# \pset border 1
Border style is 1.
rleigh=# SELECT * FROM package_priorities;
id │ name
────┼───────────
1 │ extra
2 │ important
3 │ optional
4 │ required
5 │ standard
(5 rows)
rleigh=# \pset border 2
Border style is 2.
rleigh=# SELECT * FROM package_priorities;
┌────┬───────────┐
│ id │ name │
├────┼───────────┤
│ 1 │ extra │
│ 2 │ important │
│ 3 │ optional │
│ 4 │ required │
│ 5 │ standard │
└────┴───────────┘
(5 rows)
rleigh=# \pset border 0
Border style is 0.
rleigh=# \pset expanded 1
Expanded display is on.
rleigh=# SELECT * FROM package_priorities;
─* Record 1
id 1
name extra
─* Record 2
id 2
name important
─* Record 3
id 3
name optional
─* Record 4
id 4
name required
─* Record 5
id 5
name standard
[ this might need a tiny tweak to remove the leading - ]
rleigh=# \pset border 1
Border style is 1.
rleigh=# SELECT * FROM package_priorities;
─[ RECORD 1 ]───
id │ 1
name │ extra
─[ RECORD 2 ]───
id │ 2
name │ important
─[ RECORD 3 ]───
id │ 3
name │ optional
─[ RECORD 4 ]───
id │ 4
name │ required
─[ RECORD 5 ]───
id │ 5
name │ standard
rleigh=# \pset border 2
Border style is 2.
rleigh=# SELECT * FROM package_priorities;
┌─[ RECORD 1 ]─────┐
│ id │ 1 │
│ name │ extra │
├─[ RECORD 2 ]─────┤
│ id │ 2 │
│ name │ important │
├─[ RECORD 3 ]─────┤
│ id │ 3 │
│ name │ optional │
├─[ RECORD 4 ]─────┤
│ id │ 4 │
│ name │ required │
├─[ RECORD 5 ]─────┤
│ id │ 5 │
│ name │ standard │
└──────┴───────────┘
rleigh=# \ds
List of relations
Schema │ Name │ Type │ Owner
────────┼───────────────────────────┼──────────┼────────
public │ architectures_id_seq │ sequence │ rleigh
public │ binaries_id_seq │ sequence │ rleigh
public │ components_id_seq │ sequence │ rleigh
public │ distributions_id_seq │ sequence │ rleigh
public │ package_priorities_id_seq │ sequence │ rleigh
public │ package_sections_id_seq │ sequence │ rleigh
public │ sections_id_seq │ sequence │ rleigh
public │ states_id_seq │ sequence │ rleigh
(8 rows)
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
printTextLineFormat describes the characters used to draw vertical
lines across a horizontal rule at the left side, middle and right hand
side. These are included in the formatting for an entire table
(printTextFormat). The printTextRule enum is used as an offset into
the printTextFormat line rules (lrule), allowing specification of line
styles for the top, middle and bottom horizontal lines in a table.
The other printTextFormat members, hrule and vrule define the
formatting needed to draw horizontal and vertical rules.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.h | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/src/bin/psql/print.h b/src/bin/psql/print.h
index 55122d7..ffca5d4 100644
--- a/src/bin/psql/print.h
+++ b/src/bin/psql/print.h
@@ -95,6 +95,27 @@ typedef struct printQueryOpt
* gettext on col i */
} printQueryOpt;+typedef struct printTextLineFormat
+{
+ const char *leftvrule;
+ const char *midvrule;
+ const char *rightvrule;
+} printTextLineFormat;
+
+typedef struct printTextFormat
+{
+ printTextLineFormat lrule[3];
+ const char *hrule;
+ const char *vrule;
+} printTextFormat;
+
+typedef enum printTextRule
+{
+ PRINT_RULE_TOP,
+ PRINT_RULE_MIDDLE,
+ PRINT_RULE_BOTTOM
+} printTextRule;
+ extern FILE *PageOutput(int lines, unsigned short int pager);
extern void ClosePager(FILE *pagerpipe);
--
1.6.3.3
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 7505cd4..9dec77d 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -356,6 +356,30 @@ print_unaligned_vertical(const printTableContent *cont, FILE *fout)
/* Aligned text */
/********************/+static const printTextFormat asciiformat =
+{
+ {
+ { "+", "+", "+" },
+ { "+", "+", "+" },
+ { "+", "+", "+" }
+ },
+ "-",
+ "|"
+};
+
+static const struct printTextFormat utf8format =
+{
+ {
+ /* ┌, ┬, ┐ */
+ { "\342\224\214", "\342\224\254", "\342\224\220" },
+ /* ├, ┼, ┤ */
+ { "\342\224\234", "\342\224\274", "\342\224\244" },
+ /* └, ┴, ┘ */
+ { "\342\224\224", "\342\224\264", "\342\224\230" }
+ },
+ "\342\224\200", /* ─ */
+ "\342\224\202" /* │ */
+}; /* draw "line" */
static void
--
1.6.3.3
Default to using the ASCII table. However, if a UTF-8
locale codeset is in use, switch to the UTF-8 table.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 9dec77d..6f5dcd4 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -21,6 +21,9 @@
#endif #include <locale.h>
+#ifdef HAVE_LANGINFO_H
+#include <langinfo.h>
+#endif #include "catalog/pg_type.h"
#include "pqsignal.h"
@@ -2232,7 +2235,13 @@ IsPagerNeeded(const printTableContent *cont, const int extra_lines, FILE **fout,
void
printTable(const printTableContent *cont, FILE *fout, FILE *flog)
{
- bool is_pager = false;
+ bool is_pager = false;
+ const printTextFormat *text_format = &asciiformat;
+
+#if (defined(HAVE_LANGINFO_H) && defined(CODESET))
+ if (!strcmp(nl_langinfo(CODESET), "UTF-8"))
+ text_format = &utf8format;
+#endif if (cancel_pressed)
return;
--
1.6.3.3
print_aligned_text and print_aligned_vertical, and their
helper fuctions pass the table formatting and (where
applicable) line style information to allow correct
printing of table lines.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 23 ++++++++++++++---------
1 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 6f5dcd4..641fd63 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -387,7 +387,9 @@ static const struct printTextFormat utf8format =
/* draw "line" */
static void
_print_horizontal_line(const unsigned int ncolumns, const unsigned int *widths,
- unsigned short border, FILE *fout)
+ unsigned short border, printTextRule pos,
+ const printTextFormat *format,
+ FILE *fout)
{
unsigned int i,
j;
@@ -424,7 +426,8 @@ _print_horizontal_line(const unsigned int ncolumns, const unsigned int *widths,
* Print pretty boxes around cells.
*/
static void
-print_aligned_text(const printTableContent *cont, FILE *fout)
+print_aligned_text(const printTableContent *cont, const printTextFormat *format,
+ FILE *fout)
{
bool opt_tuples_only = cont->opt->tuples_only;
bool opt_numeric_locale = cont->opt->numericLocale;
@@ -736,7 +739,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
int curr_nl_line; if (opt_border == 2)
- _print_horizontal_line(col_count, width_wrap, opt_border, fout);
+ _print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_TOP, format, fout); for (i = 0; i < col_count; i++)
pg_wcsformat((unsigned char *) cont->headers[i],
@@ -792,7 +795,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
fputc('\n', fout);
}
- _print_horizontal_line(col_count, width_wrap, opt_border, fout);
+ _print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_MIDDLE, format, fout);
}
}@@ -935,7 +938,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
if (cont->opt->stop_table)
{
if (opt_border == 2 && !cancel_pressed)
- _print_horizontal_line(col_count, width_wrap, opt_border, fout);
+ _print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_BOTTOM, format, fout); /* print footers */
if (cont->footers && !opt_tuples_only && !cancel_pressed)
@@ -970,7 +973,9 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
static void
-print_aligned_vertical(const printTableContent *cont, FILE *fout)
+print_aligned_vertical(const printTableContent *cont,
+ const printTextFormat *format,
+ FILE *fout)
{
bool opt_tuples_only = cont->opt->tuples_only;
bool opt_numeric_locale = cont->opt->numericLocale;
@@ -2258,7 +2263,7 @@ printTable(const printTableContent *cont, FILE *fout, FILE *flog)
/* print the stuff */ if (flog)
- print_aligned_text(cont, flog);
+ print_aligned_text(cont, text_format, flog); switch (cont->opt->format)
{
@@ -2271,9 +2276,9 @@ printTable(const printTableContent *cont, FILE *fout, FILE *flog)
case PRINT_ALIGNED:
case PRINT_WRAPPED:
if (cont->opt->expanded)
- print_aligned_vertical(cont, fout);
+ print_aligned_vertical(cont, text_format, fout);
else
- print_aligned_text(cont, fout);
+ print_aligned_text(cont, text_format, fout);
break;
case PRINT_HTML:
if (cont->opt->expanded)
--
1.6.3.3Convert print_aligned_text, and its helper function, to use
table formatting in place of hardcoded ASCII characters.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 50 +++++++++++++++++++++++++++++++++++++-------------
1 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 641fd63..84f6bdc 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -394,29 +394,41 @@ _print_horizontal_line(const unsigned int ncolumns, const unsigned int *widths,
unsigned int i,
j;+ const printTextLineFormat *lformat = &format->lrule[pos];
+
if (border == 1)
- fputc('-', fout);
+ fputs(format->hrule, fout);
else if (border == 2)
- fputs("+-", fout);
+ {
+ fputs(lformat->leftvrule, fout);
+ fputs(format->hrule, fout);
+ } for (i = 0; i < ncolumns; i++)
{
for (j = 0; j < widths[i]; j++)
- fputc('-', fout);
+ fputs(format->hrule, fout);
if (i < ncolumns - 1)
{
if (border == 0)
fputc(' ', fout);
else
- fputs("-+-", fout);
+ {
+ fputs(format->hrule, fout);
+ fputs(lformat->midvrule, fout);
+ fputs(format->hrule, fout);
+ }
}
}
if (border == 2)
- fputs("-+", fout);
+ {
+ fputs(format->hrule, fout);
+ fputs(lformat->rightvrule, fout);
+ }
else if (border == 1)
- fputc('-', fout);
+ fputs(format->hrule, fout); fputc('\n', fout);
}
@@ -752,7 +764,7 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format,
while (more_col_wrapping)
{
if (opt_border == 2)
- fprintf(fout, "|%c", curr_nl_line ? '+' : ' ');
+ fprintf(fout, "%s%c", format->vrule, curr_nl_line ? '+' : ' ');
else if (opt_border == 1)
fputc(curr_nl_line ? '+' : ' ', fout);@@ -783,13 +795,16 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format,
if (opt_border == 0)
fputc(curr_nl_line ? '+' : ' ', fout);
else
- fprintf(fout, " |%c", curr_nl_line ? '+' : ' ');
+ fprintf(fout, " %s%c", format->vrule, curr_nl_line ? '+' : ' ');
}
}
curr_nl_line++; if (opt_border == 2)
- fputs(" |", fout);
+ {
+ fputc(' ', fout);
+ fputs(format->vrule, fout);
+ }
else if (opt_border == 1)
fputc(' ', fout);
fputc('\n', fout);
@@ -841,7 +856,10 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format, /* left border */
if (opt_border == 2)
- fputs("| ", fout);
+ {
+ fputs(format->vrule, fout);
+ fputc(' ', fout);
+ }
else if (opt_border == 1)
fputc(' ', fout);@@ -922,14 +940,20 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format,
else if (curr_nl_line[j + 1] != 0)
fputs(" : ", fout);
else
+ {
/* Ordinary line */
- fputs(" | ", fout);
+ fputc(' ', fout);
+ fputs(format->vrule, fout);
+ fputc(' ', fout);
+ }
}
} /* end-of-row border */
- if (opt_border == 2)
- fputs(" |", fout);
+ if (opt_border == 2) {
+ fputc(' ', fout);
+ fputs(format->vrule, fout);
+ }
fputc('\n', fout); } while (more_lines);
--
1.6.3.3
Convert print_aligned_vertical, and its helper function, to use
table formatting in place of hardcoded ASCII characters.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 141 +++++++++++++++++++++++++++++++-------------------
1 files changed, 87 insertions(+), 54 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 84f6bdc..e4e9f01 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -995,6 +995,68 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format,
ClosePager(fout);
}+static inline void
+print_aligned_vertical_line(const printTableContent *cont,
+ unsigned long record,
+ unsigned int hwidth,
+ unsigned int dwidth,
+ printTextRule pos,
+ const printTextFormat *format,
+ FILE *fout)
+{
+ unsigned short opt_border = cont->opt->border;
+ unsigned int i;
+ int reclen = 0;
+ const printTextLineFormat *lformat = &format->lrule[pos];
+
+ if (opt_border == 2)
+ {
+ fputs(lformat->leftvrule, fout);
+ fputs(format->hrule, fout);
+ }
+ else
+ fputs(format->hrule, fout);
+
+ if (record)
+ {
+ if (opt_border == 0)
+ reclen = fprintf(fout, "* Record %lu", record);
+ else
+ reclen = fprintf(fout, "[ RECORD %lu ]", record);
+ }
+ if (opt_border != 2)
+ reclen++;
+ if (reclen < 0)
+ reclen = 0;
+ for (i = reclen; i < hwidth; i++)
+ fputs(opt_border > 0 ? format->hrule : " ", fout);
+ reclen -= hwidth;
+
+ if (opt_border > 0)
+ {
+ if (--reclen <= 0)
+ fputs(format->hrule, fout);
+ if (--reclen <= 0)
+ fputs(lformat->midvrule, fout);
+ if (--reclen <= 0)
+ fputs(format->hrule, fout);
+ }
+ else
+ {
+ if (reclen-- > 0)
+ fputs(" ", fout);
+ }
+ if (reclen < 0)
+ reclen = 0;
+ for (i = reclen; i < dwidth; i++)
+ fputs(opt_border > 0 ? format->hrule : " ", fout);
+ if (opt_border == 2)
+ {
+ fputs(format->hrule, fout);
+ fputs(lformat->rightvrule, fout);
+ }
+ fputc('\n', fout);
+} static void
print_aligned_vertical(const printTableContent *cont,
@@ -1014,7 +1076,6 @@ print_aligned_vertical(const printTableContent *cont,
dheight = 1,
hformatsize = 0,
dformatsize = 0;
- char *divider;
struct lineptr *hlineptr,
*dlineptr;
@@ -1082,21 +1143,6 @@ print_aligned_vertical(const printTableContent *cont,
dlineptr->ptr = pg_local_malloc(dformatsize);
hlineptr->ptr = pg_local_malloc(hformatsize);
- /* make horizontal border */
- divider = pg_local_malloc(hwidth + dwidth + 10);
- divider[0] = '\0';
- if (opt_border == 2)
- strcat(divider, "+-");
- for (i = 0; i < hwidth; i++)
- strcat(divider, opt_border > 0 ? "-" : " ");
- if (opt_border > 0)
- strcat(divider, "-+-");
- else
- strcat(divider, " ");
- for (i = 0; i < dwidth; i++)
- strcat(divider, opt_border > 0 ? "-" : " ");
- if (opt_border == 2)
- strcat(divider, "-+");
if (cont->opt->start_table)
{
@@ -1108,40 +1154,25 @@ print_aligned_vertical(const printTableContent *cont,
/* print records */
for (i = 0, ptr = cont->cells; *ptr; i++, ptr++)
{
- int line_count,
- dcomplete,
- hcomplete;
+ int line_count,
+ dcomplete,
+ hcomplete;
+ printTextRule pos = PRINT_RULE_MIDDLE;
+ if (i == 0)
+ pos = PRINT_RULE_TOP;
+ else if (!(*(ptr+1)))
+ pos = PRINT_RULE_BOTTOM;
+
+ if (cancel_pressed)
+ break; if (i % cont->ncolumns == 0)
{
- if (cancel_pressed)
- break;
- if (!opt_tuples_only)
- {
- char record_str[64];
- size_t record_str_len;
-
- if (opt_border == 0)
- snprintf(record_str, 64, "* Record %lu", record++);
- else
- snprintf(record_str, 64, "[ RECORD %lu ]", record++);
- record_str_len = strlen(record_str);
-
- if (record_str_len + opt_border > strlen(divider))
- fprintf(fout, "%.*s%s\n", opt_border, divider, record_str);
- else
- {
- char *div_copy = pg_strdup(divider);
-
- strncpy(div_copy + opt_border, record_str, record_str_len);
- fprintf(fout, "%s\n", div_copy);
- free(div_copy);
- }
- }
+ if (!opt_tuples_only)
+ print_aligned_vertical_line(cont, record++, hwidth, dwidth, pos, format, fout);
else if (i != 0 || !cont->opt->start_table || opt_border == 2)
- fprintf(fout, "%s\n", divider);
+ print_aligned_vertical_line(cont, 0, hwidth, dwidth, pos, format, fout);
}
-
/* Format the header */
pg_wcsformat((unsigned char *) cont->headers[i % cont->ncolumns],
strlen(cont->headers[i % cont->ncolumns]),
@@ -1155,7 +1186,10 @@ print_aligned_vertical(const printTableContent *cont,
while (!dcomplete || !hcomplete)
{
if (opt_border == 2)
- fputs("| ", fout);
+ {
+ fputs(format->vrule, fout);
+ fputc(' ', fout);
+ }
if (!hcomplete)
{
fprintf(fout, "%-s%*s", hlineptr[line_count].ptr,
@@ -1168,7 +1202,7 @@ print_aligned_vertical(const printTableContent *cont,
fprintf(fout, "%*s", hwidth, ""); if (opt_border > 0)
- fprintf(fout, " %c ", (line_count == 0) ? '|' : ':');
+ fprintf(fout, " %s ", (line_count == 0) ? format->vrule : ":");
else
fputs(" ", fout);@@ -1181,8 +1215,8 @@ print_aligned_vertical(const printTableContent *cont,
if (opt_border < 2)
fprintf(fout, "%s\n", my_cell);
else
- fprintf(fout, "%-s%*s |\n", my_cell,
- (int) (dwidth - strlen(my_cell)), "");
+ fprintf(fout, "%-s%*s %s\n", my_cell,
+ (int) (dwidth - strlen(my_cell)), "", format->vrule);
free(my_cell);
}
else
@@ -1190,8 +1224,8 @@ print_aligned_vertical(const printTableContent *cont,
if (opt_border < 2)
fprintf(fout, "%s\n", dlineptr[line_count].ptr);
else
- fprintf(fout, "%-s%*s |\n", dlineptr[line_count].ptr,
- dwidth - dlineptr[line_count].width, "");
+ fprintf(fout, "%-s%*s %s\n", dlineptr[line_count].ptr,
+ dwidth - dlineptr[line_count].width, "", format->vrule);
} if (!dlineptr[line_count + 1].ptr)
@@ -1202,7 +1236,7 @@ print_aligned_vertical(const printTableContent *cont,
if (opt_border < 2)
fputc('\n', fout);
else
- fprintf(fout, "%*s |\n", dwidth, "");
+ fprintf(fout, "%*s %s\n", dwidth, "", format->vrule);
}
line_count++;
}
@@ -1211,7 +1245,7 @@ print_aligned_vertical(const printTableContent *cont,
if (cont->opt->stop_table)
{
if (opt_border == 2 && !cancel_pressed)
- fprintf(fout, "%s\n", divider);
+ print_aligned_vertical_line(cont, 0, hwidth, dwidth, PRINT_RULE_BOTTOM, format, fout); /* print footers */
if (!opt_tuples_only && cont->footers != NULL && !cancel_pressed)
@@ -1227,7 +1261,6 @@ print_aligned_vertical(const printTableContent *cont,
fputc('\n', fout);
}
- free(divider);
free(hlineptr->ptr);
free(dlineptr->ptr);
free(hlineptr);
--
1.6.3.3
Small followup patch to correct expanded=1 and border=0 output
as mentioned in previous email.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index e4e9f01..be81adc 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -1014,7 +1014,7 @@ print_aligned_vertical_line(const printTableContent *cont,
fputs(lformat->leftvrule, fout);
fputs(format->hrule, fout);
}
- else
+ else if (opt_border == 1)
fputs(format->hrule, fout); if (record)
--
1.6.3.3
Further minor cleanups to tweak column alignment in a corner case,
and to correct indentation to match the rest of the code.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
Correct a corner case where the middle column separator overlaps
the right edge of the record number.
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index be81adc..c6394ad 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -1034,16 +1034,16 @@ print_aligned_vertical_line(const printTableContent *cont, if (opt_border > 0)
{
- if (--reclen <= 0)
+ if (reclen-- <= 0)
fputs(format->hrule, fout);
- if (--reclen <= 0)
+ if (reclen-- <= 0)
fputs(lformat->midvrule, fout);
- if (--reclen <= 0)
+ if (reclen-- <= 0)
fputs(format->hrule, fout);
}
else
{
- if (reclen-- > 0)
+ if (reclen-- <= 0)
fputs(" ", fout);
}
if (reclen < 0)
--
1.6.3.3
Signed-off-by: Roger Leigh <rleigh@debian.org>
---
src/bin/psql/print.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index c6394ad..10faeb3 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -1154,10 +1154,10 @@ print_aligned_vertical(const printTableContent *cont,
/* print records */
for (i = 0, ptr = cont->cells; *ptr; i++, ptr++)
{
- int line_count,
- dcomplete,
- hcomplete;
- printTextRule pos = PRINT_RULE_MIDDLE;
+ int line_count,
+ dcomplete,
+ hcomplete;
+ printTextRule pos = PRINT_RULE_MIDDLE;
if (i == 0)
pos = PRINT_RULE_TOP;
else if (!(*(ptr+1)))
--
1.6.3.3
On Sat, Aug 22, 2009 at 2:13 PM, Roger Leigh<rleigh@debian.org> wrote:
Further minor cleanups to tweak column alignment in a corner case,
and to correct indentation to match the rest of the code.
Please read the guidelines here:
http://wiki.postgresql.org/wiki/Submitting_a_Patch
I don't think it's very helpful to submit a patch intended to do
basically one thing split up over 10 threads. The PostgreSQL style is
heavyweight commits, and I don't believe that there's any reason to
suppose that someone would want to commit any one of these 9 pieces
without the other 8. So I'd suggest you resend it as a single patch
and then add it here.
https://commitfest.postgresql.org/action/commitfest_view/open
...Robert
On Sat, Aug 22, 2009 at 07:42:10PM -0400, Robert Haas wrote:
On Sat, Aug 22, 2009 at 2:13 PM, Roger Leigh<rleigh@debian.org> wrote:
Further minor cleanups to tweak column alignment in a corner case,
and to correct indentation to match the rest of the code.Please read the guidelines here:
http://wiki.postgresql.org/wiki/Submitting_a_PatchI don't think it's very helpful to submit a patch intended to do
basically one thing split up over 10 threads. The PostgreSQL style is
heavyweight commits, and I don't believe that there's any reason to
suppose that someone would want to commit any one of these 9 pieces
without the other 8.
OK. I have attached a single patch which combines the nine patches
into one. I did read the patch page above, but it didn't mention
anything on patch size, so I split it into incremental logical
changes in order to make it easily reviewable. 6-9 can be viewed
as a whole, since 7-9 are minor fixes to 6.
The other information requested on that page:
project: postgresql
patch name: psql-utf8-table-1.patch
purpose: adds support for Unicode UTF-8 box drawing to the text
table drawing functions print_aligned_text and
print_aligned_vertical.
for: discussion or application. Testing shows identical formatting
to current code.
branch: HEAD
compiles: yes
platform-specific: POSIX, but contains appropriate preprocessor
conditionals for portability. Not tested on non-POSIX platform,
but conditional taken from elsewhere (port/chklocale.c). Only
two lines of the patch are platform-specific, the rest is
plain portable C.
regression tests: No. I'm not aware of any psql regression tests
testing output formatting.
Manually confirmed output is unchanged with border=0/1/2 and
expanded=0/1.
use: Use a locale with LC_CTYPE CODESET=UTF-8.
performance: Not tested, but should be minimal effect. There's
an additional pointer dereference during string formatting
in some places. Temporary copies of data are used in some
functions e.g. of format->lrule[] for convenience and to potentially
save on the number of dereferences, though any optimising compiler
should make this cost zero (all data and pointers are const).
comments:
psql: Abstract table formatting characters used for different line types.
printTextLineFormat describes the characters used to draw vertical lines across
a horizontal rule at the left side, middle and right hand side. These are
included in the formatting for an entire table (printTextFormat). The
printTextRule enum is used as an offset into the printTextFormat line rules
(lrule), allowing specification of line styles for the top, middle and bottom
horizontal lines in a table. The other printTextFormat members, hrule and
vrule define the formatting needed to draw horizontal and vertical rules.
Add table formats for ASCII and UTF-8. Default to using the ASCII table.
However, if a UTF-8 locale codeset is in use, switch to the UTF-8 table.
print_aligned_text and print_aligned_vertical, and their helper fuctions pass
the table formatting and (where applicable) line style information to allow
correct printing of table lines.
Convert print_aligned_text, and its helper function, to use table formatting in
place of hardcoded ASCII characters. Convert print_aligned_vertical to use
table formatting in place of hardcoded ASCII characters, and add helper
function print_aligned_vertical_line to format individual lines.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
Attachments:
psql-utf8-table-1.patchtext/x-diff; charset=utf-8Download+193-77
On Sun, Aug 23, 2009 at 1:49 AM, Roger Leigh<rleigh@codelibre.net> wrote:
Please read the guidelines here:
http://wiki.postgresql.org/wiki/Submitting_a_PatchI don't think it's very helpful to submit a patch intended to do
basically one thing split up over 10 threads. The PostgreSQL style is
heavyweight commits, and I don't believe that there's any reason to
suppose that someone would want to commit any one of these 9 pieces
without the other 8.OK. I have attached a single patch which combines the nine patches
into one. I did read the patch page above, but it didn't mention
anything on patch size, so I split it into incremental logical
changes in order to make it easily reviewable. 6-9 can be viewed
as a whole, since 7-9 are minor fixes to 6.
I don't really have an opinion between one big patch versus multiple
smaller patches. That will come down to the code and whether the
separate patches are easier to read. It is sometimes hard to review a
patch when its separated from the code which needs the new
functionality though.
What I do think is getting my inbox suddenly blatted with a screenful
of patches from one author on one topicis pretty annoying. At the very
least please send all the patches as attachments to a single email. I
find it hard to understand how the linux-kernel list functions if
everyone's patches are all mixed up together and you can't save a
message and have all the related code saved but have to go back to the
list to find emails with similar names.
But the more fundamental failure here is that this email should have
been sent first. You dumped a whole pile of changes on us without ever
asking what we thought of the idea which is "not how it's done". It's
actually not as bad as it looked because it's actually not as many
changes as it looked like at first, but still.
Now I happen to like the idea of using UTF8 border characters (I
hacked emacs-w3 to use the terminal font in X to do something similar
bitd so maybe I'm biased by nostalgia though). And the changes are
actually pretty simple despite looking bigger than they really are.
They're actually only a few dozen lines.
I wonder if there's a UTF8 tab character or monospaced space character
or something which would make these tables display better in my mail
client. Currently they're completely messed up because the display
engine doesn't recognize they should be monospaced whereas the usual
output does get recognized. As a result the padding all gets collapsed
and everything is misaligned.
Greg Stark wrote:
I don't really have an opinion between one big patch versus multiple
smaller patches. That will come down to the code and whether the
separate patches are easier to read. It is sometimes hard to review a
patch when its separated from the code which needs the new
functionality though.
I have an opinion. These patches involved multiple changes to the same
few files.
Please, don't do that.
In general one patch per feature is what makes life for reviewers and
committers easier. For big features, sometimes we ask people to break it
up into several patches, but this wasn't a big feature at all, and there
is not the slightest need for more than one patch.
cheers
andrew
On Sun, Aug 23, 2009 at 01:49:02AM +0100, Roger Leigh wrote:
On Sat, Aug 22, 2009 at 07:42:10PM -0400, Robert Haas wrote:
On Sat, Aug 22, 2009 at 2:13 PM, Roger Leigh<rleigh@debian.org> wrote:
Further minor cleanups to tweak column alignment in a corner case,
and to correct indentation to match the rest of the code.Please read the guidelines here:
http://wiki.postgresql.org/wiki/Submitting_a_PatchI don't think it's very helpful to submit a patch intended to do
basically one thing split up over 10 threads. The PostgreSQL style is
heavyweight commits, and I don't believe that there's any reason to
suppose that someone would want to commit any one of these 9 pieces
without the other 8.OK. I have attached a single patch which combines the nine patches
into one. I did read the patch page above, but it didn't mention
A second patch is attached. This is identical to the previous one,
except that it's re-diffed against the REL8_4_STABLE branch for
anyone who wishes to use this with 8.4.
Again, apologies for the noise with my first set of patches.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
Attachments:
psql-utf8-table-1-8.4rel.patchtext/x-diff; charset=utf-8Download+193-77
Roger Leigh wrote:
Convert print_aligned_text, and its helper function, to use
table formatting in place of hardcoded ASCII characters.
@@ -841,7 +856,10 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format,
/* left border */ if (opt_border == 2) - fputs("| ", fout); + { + fputs(format->vrule, fout); + fputc(' ', fout); + } else if (opt_border == 1) fputc(' ', fout);
Wouldn't it be better to do a single fprintf call here instead of
fputc + fputs?
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Sun, Aug 23, 2009 at 11:47:02AM -0400, Alvaro Herrera wrote:
Roger Leigh wrote:
Convert print_aligned_text, and its helper function, to use
table formatting in place of hardcoded ASCII characters.@@ -841,7 +856,10 @@ print_aligned_text(const printTableContent *cont, const printTextFormat *format,
/* left border */ if (opt_border == 2) - fputs("| ", fout); + { + fputs(format->vrule, fout); + fputc(' ', fout); + } else if (opt_border == 1) fputc(' ', fout);Wouldn't it be better to do a single fprintf call here instead of
fputc + fputs?
It's certainly possible to change it; the above might be slightly more
efficient than a call to fprintf since you skip parsing the format
string, but otherwise I have no real preference for one over the
other.
Regards,
Roger
--
.''`. Roger Leigh
: :' : Debian GNU/Linux http://people.debian.org/~rleigh/
`. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/
`- GPG Public Key: 0x25BFB848 Please GPG sign your mail.
Roger Leigh wrote:
On Sun, Aug 23, 2009 at 11:47:02AM -0400, Alvaro Herrera wrote:
Wouldn't it be better to do a single fprintf call here instead of
fputc + fputs?It's certainly possible to change it; the above might be slightly more
efficient than a call to fprintf since you skip parsing the format
string, but otherwise I have no real preference for one over the
other.
There are already other fprintf calls in the same function -- I wouldn't
worry too much about some hypothetical performance gain. Let's keep
things simple and readable.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
