Re: WIP: psql default banner patch v4

On Wed, 23 Apr 2008 17:44:43 -0400
Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:

> Joshua D. Drake wrote:
>
> > ! puts(_("\n"));
> > ! puts(_("You are using psql, the
> > command-line interface to PostgreSQL.\n")); !
> > puts(_("\tFor SQL help type \\h or
> > \\help ."));
> ^
> here
> > ! puts(_("\tFor help using psql type
> > \\? ."));
> ^ here
> > ! puts(_("\tTo quit psql type \\q .\n"));
> ^ here
> > ! puts(_("\tTo view the copyright type
> > \\copyright .\n"));

Ahh o.k. Now I have a complaint. :) I happily removed the whitespace
where I saw this, "%s \n" (for example) but the whitespace above is for
readability. Consider:

To quit psql type \q.

I am trying to avoid the silly newbie saying, "I typed \q. and it did
nothing".

Maybe I am being overly cautious?

> > --- 158,164 ----
> > /* DB server user name */
> > case 'n':
> > if (pset.db)
> > ! strlcpy(buf,
> > session_username(), sizeof(buf)); break;
> >
> > case '0':
>
> Please remove this hunk. (In general make sure there are no useless
> hunks in the diff.)

Well to be honest, I wouldn't have known it was useless as I didn't
write or purposely modify that part of the code.

>
>
> > if (pset.sversion / 100 != client_ver /
> > 100) ! printf(_("\tWARNING: Server
> > version %d.%d, %s version %d.%d.\n\tSome psql features may not
> > work.\n\n"),
>
>
> Minor suggestion: it looks better this way (the end effect is the
> same):
>
> printf(_("\tWARNING: Server version
> %d.%d, %s version %d.%d.\n" "\tSome psql features may not work.\n\n"),

You are right. I will change that.

SIncerely,

Joshua D. Drake

--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate