WIP: psql default banner patch

Hello,

Attached is a patch for the default message from psql. Couple of things
of note:

1. I removed the force wrapping, instead allowing the terminal to do
it. This seems to work in X and well as on the console. I am not sure
if I like this or not and am not opposed to changing to a forced wrap.

2. I significantly reduced the wording making the intro more of a
notice than a welcome message.

3. I added a new line to help.c which stands for Execution. I read
the \? help three times before finding \g in the Query Buffer section.
That doesn't seem to me to be a good sign. I left the \g in Query
Buffer as well (yes its redundant) because it shows other options for
use with \g.

Please let me know what you think.

Sincerely,

Joshau 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

On Mon, 21 Apr 2008 13:17:40 -0700
"Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:

Hello,

Attached is v2 of this patch. The following changes have been made:

* If there is not a version mismatch, psql tells you nothing but ask
for help if you need it.
* If there is a version mismatch it tells you and still tells you to
type help if you need it

I have modified mainloop.c to deal with help in this instance.

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

Joshua D. Drake wrote:

> Attached is v2 of this patch. The following changes have been made:
>
> * If there is not a version mismatch, psql tells you nothing but ask
> for help if you need it.
> * If there is a version mismatch it tells you and still tells you to
> type help if you need it

Thanks.

> I have modified mainloop.c to deal with help in this instance.

I must admit I don't like the new wording in this patch. There are
extraneous spaces, which I guess are there just because you don't want
to put the \\X command immediately followed by the sentence-finishing
dot. The original wording avoided that just by having the \\X command
away from the end of the sentence.

> ! puts(_("\tTo view the copyright type \\c . \n"));

The copyright is show with \copyright, not \c.

> if (pset.sversion / 100 != client_ver / 100)
> ! printf(_("\nWARNING: Server %d.%d, %s is version %d.%d. Some psql features may not work.\n\n"),
> ! pset.sversion / 10000, (pset.sversion / 100) % 100,
> pset.progname,
> client_ver / 10000, (client_ver / 100) % 100);

I think the warning should be two lines:

WARNING: Server is version %d.%d, %s is version %d.%d.
Some backslash commands may not work.

> + printf(_("Type: help for help. \n"));
> +

Here you have a pointless extraneous space.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Tue, 22 Apr 2008 17:56:49 -0400
Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:

> > ! puts(_("\tTo view the copyright type
> > \\c . \n"));
>
> The copyright is show with \copyright, not \c.

Doh! I knew that. I will fix after the below discussion :)

>
> I think the warning should be two lines:
>
> WARNING: Server is version %d.%d, %s is version %d.%d.
> Some backslash commands may not work.

I tried that, I thought it looked weird. The single line fits within
the 80 character wrap point. Do we really want to create two lines?

>
>
> > + printf(_("Type: help for help. \n"));
> > +
>
> Here you have a pointless extraneous space.
>

Not that I care but does it matter? I mean it is a space before a
newline.

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

Joshua D. Drake wrote:
> On Tue, 22 Apr 2008 17:56:49 -0400
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
>
> > I think the warning should be two lines:
> >
> > WARNING: Server is version %d.%d, %s is version %d.%d.
> > Some backslash commands may not work.
>
> I tried that, I thought it looked weird. The single line fits within
> the 80 character wrap point. Do we really want to create two lines?

Yes IMHO. The problem is that on a translation the line may be longer.
(In fact, on most western languages other than english, it will be
longer.) Of course, the translator can fix it for himself, but in
practice what most translators do is follow whatever original newline
convention there is, so it's better that there is some slack space.

> > > + printf(_("Type: help for help. \n"));
> > > +
> >
> > Here you have a pointless extraneous space.
>
> Not that I care but does it matter? I mean it is a space before a
> newline.

It's not very important, but again for translations it's better than the
whole thing is whitespace-clean.

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

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> * If there is not a version mismatch, psql tells you nothing but ask
> for help if you need it.

That was NOT part of the agreement. The version line should stay.

regards, tom lane

On Tue, 22 Apr 2008 19:13:54 -0400
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> "Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> > * If there is not a version mismatch, psql tells you nothing but
> > ask for help if you need it.
>
> That was NOT part of the agreement. The version line should stay.

Why do we care, if the version matches? Not that I am feeling like
fighting about it but it seems just a waste of bytes. It makes sense if
the version doesn't match.

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

On Tue, 22 Apr 2008 16:28:20 -0700
"Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:

> Why do we care, if the version matches? Not that I am feeling like
> fighting about it but it seems just a waste of bytes. It makes sense
> if the version doesn't match.
>
> Joshua D. Drake

Actually in thinking about this, this is what the patch does now:

If version matches:

jd(at)scratch:~/pgsql/src/bin/psql$ ./psql -U jd -h localhost -p 9000
postgres Type: help for help.
postgres=#

If version doesn't match:

jd(at)scratch:~/pgsql/src/bin/psql$ ./psql -U jd -h localhost postgres

WARNING: Server 8.2, psql is version 8.4. Some psql features may not
work.

Type: help for help.
postgres=>

I understand Alvaro's point on the translation so no sweat, I will fix
that. What if I add the version to the help? With this patch if you
type help you get:

postgres=> help

You are using psql, the command-line interface to PostgreSQL.

For SQL help type \h or \help .
For help using psql type \? .
To quit psql type \q .

To view the copyright type \copyright .

postgres=>

I could change that to:

You are using psql 8.2.7, the command-line interface to PostgreSQL.

For SQL help type \h or \help .
For help using psql type \? .
To quit psql type \q .

To view the copyright type \copyright .

Does that make sense?

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

Joshua D. Drake wrote:
> On Tue, 22 Apr 2008 19:13:54 -0400
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
>
>> "Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
>>
>>> * If there is not a version mismatch, psql tells you nothing but
>>> ask for help if you need it.
>>>
>> That was NOT part of the agreement. The version line should stay.
>>
>
> Why do we care, if the version matches?
>

You might care if you are working with more than one version at once,
even if the psql you're using matches the server it's talking to.
(That's kinda why I suggested the version as a possible prompt escape)

cheers

andrew

On Tue, 22 Apr 2008 19:56:53 -0400
Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

>
> You might care if you are working with more than one version at once,
> even if the psql you're using matches the server it's talking to.
> (That's kinda why I suggested the version as a possible prompt escape)

O.k. I don't know what you mean (not to sound dumb). Do you mean a: \V
which would give the version? That would be cool.

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, Apr 23, 2008 at 9:28 AM, Joshua D. Drake wrote:
> Why do we care, if the version matches? Not that I am feeling like
> fighting about it but it seems just a waste of bytes. It makes sense if
> the version doesn't match.
>

To take a field example, I have one site where I run an 8.2 production
cluster, an 8.3 application testing cluster and my a personal 8.4devel
cluster for hacking postgres.

In such an environment, it's *very* useful to have instant feedback on
which server I've just connected to with psql.

Cheers,
BJ
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIDn3R5YBsbHkuyV0RAjQKAKCLlD3OIdk53D9gI8Kr0BI75WsRSwCfRQLB
30F55MfJN0W6Lzo8mba/yvg=
=fQzl
-----END PGP SIGNATURE-----

Joshua D. Drake wrote:
> On Tue, 22 Apr 2008 19:56:53 -0400
> Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
>> You might care if you are working with more than one version at once,
>> even if the psql you're using matches the server it's talking to.
>> (That's kinda why I suggested the version as a possible prompt escape)
>>
>
> O.k. I don't know what you mean (not to sound dumb). Do you mean a: \V
> which would give the version? That would be cool.
>
>
>
No, I mean you could put it in your psql prompt.

8.3 dbname>

cheers

andrew

On Wed, 23 Apr 2008 10:07:46 +1000
"Brendan Jurd" <direvus(at)gmail(dot)com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Wed, Apr 23, 2008 at 9:28 AM, Joshua D. Drake wrote:
> > Why do we care, if the version matches? Not that I am feeling like
> > fighting about it but it seems just a waste of bytes. It makes
> > sense if the version doesn't match.
> >
>
> To take a field example, I have one site where I run an 8.2 production
> cluster, an 8.3 application testing cluster and my a personal 8.4devel
> cluster for hacking postgres.
>
> In such an environment, it's *very* useful to have instant feedback on
> which server I've just connected to with psql.

The patch as it sits would provide you with that. The only time it
doesn't give you that info is if the version matches.

How do you feel about a \V that allows you to output 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

On Tue, 22 Apr 2008 20:09:44 -0400
Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:

> >
> No, I mean you could put it in your psql prompt.
>
> 8.3 dbname>

Oh that's interesting. Let me see how it looks.

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, Apr 23, 2008 at 10:11 AM, Joshua D. Drake wrote:
> On Wed, 23 Apr 2008 10:07:46 +1000
> "Brendan Jurd" wrote:
> > In such an environment, it's *very* useful to have instant feedback on
> > which server I've just connected to with psql.
>
> The patch as it sits would provide you with that. The only time it
> doesn't give you that info is if the version matches.
>

That's just it, I would want the banner to tell me the server and
client version in the same way, every time. This information doesn't
stop being interesting when the versions happen to match.

With your proposal, when the banner goes silent about the version, I
need to think "Oh yeah, that means that the server version is the same
as the psql version. Umm, what version of psql did I execute again?"
Bearing in mind that I might be executing any one of several psqls I
might have installed at any given time, and that the versions I have
installed will differ, depending on which machine I happen to be on.

Cheers,
BJ
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIDoHt5YBsbHkuyV0RAjVJAJ0XU8QBzfgomy29gxNtAjltNB+7QQCgofUG
4dJJFnrrX9yYKwDwNbD+sy4=
=w+7D
-----END PGP SIGNATURE-----

On Wed, 23 Apr 2008 10:25:19 +1000
"Brendan Jurd" <direvus(at)gmail(dot)com> wrote:

> With your proposal, when the banner goes silent about the version, I
> need to think "Oh yeah, that means that the server version is the same
> as the psql version. Umm, what version of psql did I execute again?"
> Bearing in mind that I might be executing any one of several psqls I
> might have installed at any given time, and that the versions I have
> installed will differ, depending on which machine I happen to be on.

Sure. What did you think about Andrew's suggestion?

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, Apr 23, 2008 at 10:37 AM, Joshua D. Drake wrote:
> Sure. What did you think about Andrew's suggestion?

I think it's cool; allowing the version to be added to a custom psql
prompt is a nice feature, regardless of how we go with the banner.
I'd still like to see the version numbers in the banner in all cases
though =)

For my usage patterns, once per session is often enough to be reminded
of the versions. Once per statement (as you'd get from having it in
the prompt) would be overkill.

Cheers,
BJ
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIDoci5YBsbHkuyV0RAvKYAJ9bUg5UeLKjNoK2/v+Vi9aionmUDwCgmQ9T
mcGaOBCStKJKFcuDQFgk5pU=
=ZitH
-----END PGP SIGNATURE-----

On Wed, 23 Apr 2008 10:47:32 +1000
"Brendan Jurd" <direvus(at)gmail(dot)com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Wed, Apr 23, 2008 at 10:37 AM, Joshua D. Drake wrote:
> > Sure. What did you think about Andrew's suggestion?
>
> I think it's cool; allowing the version to be added to a custom psql
> prompt is a nice feature, regardless of how we go with the banner.
> I'd still like to see the version numbers in the banner in all cases
> though =)
>
> For my usage patterns, once per session is often enough to be reminded
> of the versions. Once per statement (as you'd get from having it in
> the prompt) would be overkill.

O.k. I am not really arguing strongly one way or the other. This is
more an educational exercise for me. However, as someone who is
constantly on dozens of postgresql versions every day, I honestly fail
to see the benefit here. I get that you "like" the idea, what I don't
get is the appreciable benefit to having the version in a matching
system. What do I gain by knowing that I attached to 8.2.4 of psql with
8.2.6 of postgresql?

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

> > > * If there is not a version mismatch, psql tells you nothing but
> > > ask for help if you need it.
> >
> > That was NOT part of the agreement. The version line should stay.
>
> Why do we care, if the version matches? Not that I am feeling like
> fighting about it but it seems just a waste of bytes. It

I think the idea is that the person using psql needs to do things
differently
for different versions. So it spares him from typing an extra command to
know
how to proceed.

+1 for keep version

I personally would prefer to see both psql and server version even if
identical.
This would allow a uniform look and feel.

Andreas

Hello,

O.k. here is version 3 of the patch.

It is the same patch except that on standard connect it emits the
client version. It does not emit the server version unless their is a
mismatch. Does that make sense?

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

On Wed, 23 Apr 2008 09:24:42 -0700
"Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:

> Hello,
>
> O.k. here is version 3 of the patch.
>
> It is the same patch except that on standard connect it emits the
> client version. It does not emit the server version unless their is a
> mismatch. Does that make sense?

Oh and just a mention, I will look at the prompt stuff after the next
commit fest.

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

Joshua D. Drake wrote:

> O.k. here is version 3 of the patch.
>
> It is the same patch except that on standard connect it emits the
> client version. It does not emit the server version unless their is a
> mismatch. Does that make sense?

But you didn't fix any of the whitespace issues I mentioned ...

Also, I don't think there's anyone pushing for not showing the version
at normal start, except you.

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

On Wed, 23 Apr 2008 12:38:22 -0400
Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:

> Joshua D. Drake wrote:
>
> > O.k. here is version 3 of the patch.
> >
> > It is the same patch except that on standard connect it emits the
> > client version. It does not emit the server version unless their is
> > a mismatch. Does that make sense?
>
> But you didn't fix any of the whitespace issues I mentioned ...

Yes I did. The warning line looks like:

jd(at)scratch:~/pgsql/src/bin/psql$ ./psql -U jd -h localhost postgres

psql 8.4devel

WARNING: Server version 8.2, psql version 8.4.
psql features may not work.

Type: help for help.

I did add tabs so the message would stand out more but that isn't
really any different than the old banner message.

>
> Also, I don't think there's anyone pushing for not showing the version
> at normal start, except you.
>

The version is shown:

jd(at)scratch:~/pgsql/src/bin/psql$ ./psql -U jd -h localhost -p9000
postgres

psql 8.4devel

Type: help for help.
postgres=#

I left off server version because there doesn't seem to be a reason to
have it except if the server doesn't match. For example, Benjamin's
comment was:

--
To take a field example, I have one site where I run an 8.2 production
cluster, an 8.3 application testing cluster and my a personal 8.4devel
cluster for hacking postgres.

In such an environment, it's *very* useful to have instant feedback on
which server I've just connected to with psql.
--

Which this patch does supply.

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

> WARNING: Server version 8.2, psql version 8.4.
> psql features may not work.

Can we say "Some psql features..."? the lowercase looks odd.

> I left off server version because there doesn't seem to be a
> reason to have it except if the server doesn't match.

I'd like to have both for consistency's sake. It also helps if you're
debugging over the phone or looking at a pasted section. It also
makes the warning message easier, as there is no need to embed
version numbers inside of it.

If I had enough time, I'd start just making psql backwards-compatible,
but that's another thread...

- --
Greg Sabino Mullane greg(at)turnstep(dot)com
End Point Corporation
PGP Key: 0x14964AC8 200804231325
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAkgPcXsACgkQvJuQZxSWSsgg/wCeIFEwHmLcJj3ItkPaadsR/Hfw
iz4AoIVIF633ZWyj8mwk8LvKi00AMtMZ
=lVMb
-----END PGP SIGNATURE-----

On Wed, 23 Apr 2008 17:28:04 -0000
"Greg Sabino Mullane" <greg(at)turnstep(dot)com> wrote:

>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: RIPEMD160
>
>
> > WARNING: Server version 8.2, psql version 8.4.
> > psql features may not work.
>
> Can we say "Some psql features..."? the lowercase looks odd.

Yep. I struggled with that through every version of the patch because
the wording is weird as a whole, imo.

Some psql features may not work.
Specialized features may not work?
Version specific features may not work?

etc....

Some is fine. :)

>
> > I left off server version because there doesn't seem to be a
> > reason to have it except if the server doesn't match.
>
> I'd like to have both for consistency's sake. It also helps if you're
> debugging over the phone or looking at a pasted section. It also
> makes the warning message easier, as there is no need to embed
> version numbers inside of it.

O.k. I can buy this argument. I guess if they are running into a bug in
8.1.4 it is an easy way to know.

What are people's thoughts on:

Instead of having all this static info just doing a:

psql <version>
PostgreSQL 8.2.7 on x86_64-pc-linux-gnu,
compiled by GCC cc (GCC) 4.2.3 (Ubuntu 4.2.3-2ubuntu4)

Type: help for help.
postgres=>

>
> If I had enough time, I'd start just making psql backwards-compatible,
> but that's another thread...

oh good lord ... :P

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

On Wed, 23 Apr 2008 10:48:24 -0700
"Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:

O.k. I *think* this should do it.

Added word Some
Version shows up no matter what
Server version shows up only if:
major or minor version doesn't match (same same as previous behavior)

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

Joshua D. Drake wrote:
> On Wed, 23 Apr 2008 10:48:24 -0700
> "Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:
>
> O.k. I *think* this should do it.

Whitespace still broken

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

> Joshua D. Drake wrote:
> > On Wed, 23 Apr 2008 10:48:24 -0700
> > "Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:
> >
> > O.k. I *think* this should do it.
>
> Whitespace still broken

Well maybe if we just declared that English is *The* PostgreSQL
language, this wouldn't be a problem :P.

New patch attached, if whitespace is still broken you will need to tell
me where.

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

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"));
^ here

> Index: prompt.c
> ===================================================================
> RCS file: /projects/cvsroot/pgsql/src/bin/psql/prompt.c,v
> retrieving revision 1.51
> diff -c -r1.51 prompt.c
> *** prompt.c 1 Jan 2008 19:45:56 -0000 1.51
> --- prompt.c 23 Apr 2008 21:32:20 -0000
> ***************
> *** 158,164 ****
> /* DB server user name */
> case 'n':
> if (pset.db)
> ! strlcpy(buf, session_username(), sizeof(buf));
> break;
>
> case '0':
> --- 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.)

> 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"),

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

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

Joshua D. Drake wrote:
> On Wed, 23 Apr 2008 17:44:43 -0400
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> wrote:
>
> > Joshua D. Drake wrote:
>
> 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".

I know, which is why I suggested rewording the messages so that the
command is somewhere other than the end of the sentence. (This was
about 4 messages back in the thread).

> Maybe I am being overly cautious?

No, I think it's a reasonable thing to consider.

> > > --- 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.

Right. I suggest you open the file in "meld" which allows you to easily
remove the offending extraneous difference. Of course, you can do it in
Vim or Emacs directly, but I don't think Joe can do anything of the sort
... but we're not about to enter another editor flamewar, now, are we?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

> Right. I suggest you open the file in "meld" which allows you to
> easily remove the offending extraneous difference. Of course, you
> can do it in Vim or Emacs directly, but I don't think Joe can do
> anything of the sort ... but we're not about to enter another editor
> flamewar, now, are we?

I am quite comfortable in the superiority of my editor over yours. :P

Attached is a new patch, I cleaned up that whitespace by changing the
wording. I put the commands in the front of the sentence. I also
removed the unneeded hunks and changed the formatting of the printf().

If this is acceptable I will send it to the patches list as an update.

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

Joshua D. Drake wrote:
> On Wed, 23 Apr 2008 09:24:42 -0700
> "Joshua D. Drake" <jd(at)commandprompt(dot)com> wrote:
>
> > Hello,
> >
> > O.k. here is version 3 of the patch.
> >
> > It is the same patch except that on standard connect it emits the
> > client version. It does not emit the server version unless their is a
> > mismatch. Does that make sense?
>
> Oh and just a mention, I will look at the prompt stuff after the next
> commit fest.

Added to psql TODO:

o Add prompt escape to display the client and server versions

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Am Mittwoch, 23. April 2008 schrieb Andrew Dunstan:
> No, I mean you could put it in your psql prompt.
>
> 8.3 dbname>

You can put variables in your prompt. The VERSION variable exists, but it is
a bit verbose. Perhaps it can be trimmed.