Re: Proposed patch - psql wraps at window width - Mailing list pgsql-patches

From Alvaro Herrera
Subject Re: Proposed patch - psql wraps at window width
Date
Msg-id 20080417205307.GT3846@alvh.no-ip.org
Whole thread Raw
In response to Proposed patch - psql wraps at window width  (Bryce Nesbitt <bryce2@obviously.com>)
Responses Re: Proposed patch - psql wraps at window width
Re: Proposed patch - psql wraps at window width
List pgsql-patches
Bryce Nesbitt wrote:
> I've attached a patch, against current 8.4 cvs, which optionally sets a
> maximum width for psql output:

Some random comments:

* Don't use C++ style comments (//).  Some compilers don't like these.

* Beware of brace position: we use braces on their own, indented at the
  start of a new line, so

!     while(--count) {
!         lines++;
!         lines->ptr   = NULL;
!         lines->width = 0;
!         }

becomes


!     while(--count)
!       {
!         lines++;
!         lines->ptr   = NULL;
!         lines->width = 0;
!         }

(with correct indentation anyway)


* Always use tabs, not spaces, to indent.  Tabs are 4 spaces wide.

* Don't use double stars in comments.

* We're not in the habit of giving credit in code comments.  It gets
messy fast.

* Don't lose warning comments like this one (unless you've removed the
assumption of course)

/*
 * Assumption: This code used only on strings
 * without multibyte characters, otherwise
 * this_line->width < strlen(this_ptr) and we get
 * an overflow
 */

In fact I wonder if you've introduced this assumption in the other case
on that code (i.e. when alignment is not 'r').  I'm not seeing any
checks for multibytes in there, but perhaps I'm missing it.


* "} else" is forbidden too.  Use two separate lines.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

pgsql-patches by date:

Previous
From: "Brendan Jurd"
Date:
Subject: Re: [HACKERS] Multiline privileges in \z
Next
From: Bruce Momjian
Date:
Subject: Re: Proposed patch - psql wraps at window width