Re: WIP: psql default banner patch v4 - Mailing list pgsql-hackers

From Joshua D. Drake
Subject Re: WIP: psql default banner patch v4
Date
Msg-id 20080423144909.092e8c4c@commandprompt.com
Whole thread Raw
In response to Re: WIP: psql default banner patch v4  (Alvaro Herrera <alvherre@commandprompt.com>)
Responses Re: WIP: psql default banner patch v4  (Alvaro Herrera <alvherre@commandprompt.com>)
List pgsql-hackers
On Wed, 23 Apr 2008 17:44:43 -0400
Alvaro Herrera <alvherre@commandprompt.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




pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: WIP: psql default banner patch v4
Next
From: Alvaro Herrera
Date:
Subject: Re: WIP: psql default banner patch v4