On Sat, Aug 22, 2009 at 07:42:10PM -0400, Robert Haas wrote:
> On Sat, Aug 22, 2009 at 2:13 PM, Roger Leigh<rle...@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.

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.
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index 7505cd4..10faeb3 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"
@@ -356,38 +359,76 @@ 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
 _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;
 
+	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);
 }
@@ -397,7 +438,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;
@@ -709,7 +751,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],
@@ -722,7 +764,7 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
 			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);
 
@@ -753,19 +795,22 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
 						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);
 			}
 
-			_print_horizontal_line(col_count, width_wrap, opt_border, fout);
+			_print_horizontal_line(col_count, width_wrap, opt_border, PRINT_RULE_MIDDLE, format, fout);
 		}
 	}
 
@@ -811,7 +856,10 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
 
 			/* left border */
 			if (opt_border == 2)
-				fputs("| ", fout);
+			{
+				fputs(format->vrule, fout);
+				fputc(' ', fout);
+			}
 			else if (opt_border == 1)
 				fputc(' ', fout);
 
@@ -892,14 +940,20 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
 					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);
@@ -908,7 +962,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)
@@ -941,9 +995,73 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
 		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 if (opt_border == 1)
+		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, 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;
@@ -958,7 +1076,6 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 				dheight = 1,
 				hformatsize = 0,
 				dformatsize = 0;
-	char	   *divider;
 	struct lineptr *hlineptr,
 			   *dlineptr;
 
@@ -1026,21 +1143,6 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 	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)
 	{
@@ -1052,40 +1154,25 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 	/* 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]),
@@ -1099,7 +1186,10 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 		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,
@@ -1112,7 +1202,7 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 				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);
 
@@ -1125,8 +1215,8 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 					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
@@ -1134,8 +1224,8 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 					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)
@@ -1146,7 +1236,7 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 				if (opt_border < 2)
 					fputc('\n', fout);
 				else
-					fprintf(fout, "%*s |\n", dwidth, "");
+					fprintf(fout, "%*s %s\n", dwidth, "", format->vrule);
 			}
 			line_count++;
 		}
@@ -1155,7 +1245,7 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 	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)
@@ -1171,7 +1261,6 @@ print_aligned_vertical(const printTableContent *cont, FILE *fout)
 		fputc('\n', fout);
 	}
 
-	free(divider);
 	free(hlineptr->ptr);
 	free(dlineptr->ptr);
 	free(hlineptr);
@@ -2208,7 +2297,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;
@@ -2225,7 +2320,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)
 	{
@@ -2238,9 +2333,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)
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);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to