Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Ah, OK. I had forgotten. Here is the new output:
>
> > $ sql test
> > psql (8.4devel) Type "help" for help.
>
> > test=> help
>
> You are being unreasonably cryptic here. What happens when there
> is optional output --- ie, version mismatch warning and/or SSL info?

Oh, good point. Let me look at that. Thanks. You prefer:

$ sql test
psql (8.4devel)
Type "help" for help.

test=> help

That looked so sparse to me.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Bruce Momjian <bruce@momjian.us> writes:
> Oh, good point. Let me look at that. Thanks. You prefer:

> $ sql test
> psql (8.4devel)
> Type "help" for help.

> test=> help

Well, the question is still "where is the optional info going to go?"

I think what I'd find nice looking is

$ psql test
psql 8.4devel [ server version warning here, if needed ]
[ line with SSL info here, if needed ]
Type "help" for help.

test=>

I do feel that the help statement ought to be on its own line;
the other way is going to look cluttered, particularly as soon
as there's a version warning in there.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Tom Lane wrote:

> Well, the question is still "where is the optional info going to go?"
>
> I think what I'd find nice looking is
>
> $ psql test
> psql 8.4devel [ server version warning here, if needed ]
> [ line with SSL info here, if needed ]
> Type "help" for help.
>
> test=>
>
> I do feel that the help statement ought to be on its own line;
> the other way is going to look cluttered, particularly as soon
> as there's a version warning in there.

O.k. I am not trying to start an argument here but... I already sent 6
revisions of this patch that received comments and had thorough review
via Alvaro. I even took into account Tom's original comments from the
previous thread.

This much effort on something so simple makes it not worth the effort in
the first place.

Bruce with respect the only useful thing I have seen you do to the patch
in all this wrangling is realign the \? General options and frankly even
that is suspect in my opinion. Can we either throw it away and say,
"Nice try JD" or just commit the thing.

Joshua D. Drake


>
> regards, tom lane
>


--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Joshua D. Drake wrote:
> O.k. I am not trying to start an argument here but... I already sent 6
> revisions of this patch that received comments and had thorough review
> via Alvaro. I even took into account Tom's original comments from the
> previous thread.
>
> This much effort on something so simple makes it not worth the effort in
> the first place.
>
> Bruce with respect the only useful thing I have seen you do to the patch
> in all this wrangling is realign the \? General options and frankly even
> that is suspect in my opinion. Can we either throw it away and say,
> "Nice try JD" or just commit the thing.

Your patch is getting the same review any other patch would have. If
you want someone else to apply it I will stop working on it.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Andrew Dunstan wrote:

> Welcome to UI development. There is always *far* more argument of minor
> matters of appearance than over anything else, in my experience.

Which is a good thing (in this case at least), because otherwise we
would end up with a crappy UI just because a single person thinks it's
"good enough".

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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > Oh, good point. Let me look at that. Thanks. You prefer:
>
> > $ sql test
> > psql (8.4devel)
> > Type "help" for help.
>
> > test=> help
>
> Well, the question is still "where is the optional info going to go?"
>
> I think what I'd find nice looking is
>
> $ psql test
> psql 8.4devel [ server version warning here, if needed ]
> [ line with SSL info here, if needed ]
> Type "help" for help.
>
> test=>
>
> I do feel that the help statement ought to be on its own line;
> the other way is going to look cluttered, particularly as soon
> as there's a version warning in there.

OK, here is the normal startup now:

$ sql test
psql (8.4.0)
Type "help" for help.

test=>

Here is a minor version mismatch:

$ sql test
psql (8.4.0, server 8.4.1)
Type "help" for help.

test=>

Here is a major version mismatch:

$ sql test
psql (8.4.0, server 8.3.1)
WARNING: psql version 8.4.0, server version 8.3.1.
Some psql features might not work.
Type "help" for help.

test=>

I have also added '\g' to the 'help' display:

test=> help

You are using psql, the command-line interface to PostgreSQL.
\? for psql help
\h or \help for SQL help

\g or ";" to execute a query
\q to quit psql

\copyright to view the copyright

test=>

I don't know how to generate an SSL message.

With the new smaller \? "General" section, I though it was worth
considering if we still want to do the help banner the same. Obviously
we do, but I wanted to explore it.

Patch attached.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Index: src/bin/psql/help.c
================================================== =================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.127
diff -c -c -r1.127 help.c
*** src/bin/psql/help.c 14 May 2008 15:30:22 -0000 1.127
--- src/bin/psql/help.c 15 May 2008 16:05:51 -0000
***************
*** 170,182 ****
*/
fprintf(output, _("General\n"));
fprintf(output, _(" \\copyright show PostgreSQL usage and distribution terms\n"));
fprintf(output, _(" \\h [NAME] help on syntax of SQL commands, * for all commands\n"));
fprintf(output, _(" \\q quit psql\n"));
fprintf(output, "\n");

fprintf(output, _("Query Buffer\n"));
fprintf(output, _(" \\e [FILE] edit the query buffer (or file) with external editor\n"));
- fprintf(output, _(" \\g [FILE] send query buffer to server (and results to file or |pipe)\n"));
fprintf(output, _(" \\p show the contents of the query buffer\n"));
fprintf(output, _(" \\r reset (clear) the query buffer\n"));
#ifdef USE_READLINE
--- 170,182 ----
*/
fprintf(output, _("General\n"));
fprintf(output, _(" \\copyright show PostgreSQL usage and distribution terms\n"));
+ fprintf(output, _(" \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
fprintf(output, _(" \\h [NAME] help on syntax of SQL commands, * for all commands\n"));
fprintf(output, _(" \\q quit psql\n"));
fprintf(output, "\n");

fprintf(output, _("Query Buffer\n"));
fprintf(output, _(" \\e [FILE] edit the query buffer (or file) with external editor\n"));
fprintf(output, _(" \\p show the contents of the query buffer\n"));
fprintf(output, _(" \\r reset (clear) the query buffer\n"));
#ifdef USE_READLINE
Index: src/bin/psql/mainloop.c
================================================== =================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.90
diff -c -c -r1.90 mainloop.c
*** src/bin/psql/mainloop.c 5 Apr 2008 03:40:15 -0000 1.90
--- src/bin/psql/mainloop.c 15 May 2008 16:05:51 -0000
***************
*** 177,186 ****
(line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
{
free(line);
! puts(_("You are using psql, the command-line interface to PostgreSQL."));
! puts(_("Enter SQL commands, or type \\? for a list of backslash options."));
! puts(_("Use \\h for SQL command help."));
! puts(_("Use \\q to quit."));
fflush(stdout);
continue;
}
--- 177,189 ----
(line[4] == '\0' || line[4] == ';' || isspace((unsigned char) line[4])))
{
free(line);
! puts(_("\nYou are using psql, the command-line interface to PostgreSQL."));
! puts(_("\t\\? for psql help"));
! puts(_("\t\\h or \\help for SQL help\n"));
! puts(_("\t\\g or \";\" to execute a query"));
! puts(_("\t\\q to quit psql\n"));
! puts(_("\t\\copyright to view the copyright\n"));
!
fflush(stdout);
continue;
}
Index: src/bin/psql/startup.c
================================================== =================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.147
diff -c -c -r1.147 startup.c
*** src/bin/psql/startup.c 8 May 2008 17:04:26 -0000 1.147
--- src/bin/psql/startup.c 15 May 2008 16:05:52 -0000
***************
*** 317,342 ****
server_version = server_ver_str;
}

! printf(_("Welcome to %s %s (server %s), the PostgreSQL interactive terminal.\n\n"),
! pset.progname, PG_VERSION, server_version);
}
else
! printf(_("Welcome to %s %s, the PostgreSQL interactive terminal.\n\n"),
! pset.progname, PG_VERSION);
!
! printf(_("Type: \\copyright for distribution terms\n"
! " \\h for help with SQL commands\n"
! " \\? for help with psql commands\n"
! " \\g or terminate with semicolon to execute query\n"
! " \\q to quit\n\n"));

if (pset.sversion / 100 != client_ver / 100)
! printf(_("WARNING: You are connected to a server with major version %d.%d,\n"
! "but your %s client is major version %d.%d. Some backslash commands,\n"
! "such as \\d, might not work properly.\n\n"),
! pset.sversion / 10000, (pset.sversion / 100) % 100,
! pset.progname,
! client_ver / 10000, (client_ver / 100) % 100);

#ifdef USE_SSL
printSSLInfo();
--- 317,335 ----
server_version = server_ver_str;
}

! printf(_("%s (%s, server %s)\n"),
! pset.progname, PG_VERSION, server_version);
}
else
! printf("%s (%s)\n", pset.progname, PG_VERSION);

if (pset.sversion / 100 != client_ver / 100)
! printf(_("\tWARNING: %s version %d.%d, server version %d.%d.\n"
! "\tSome psql features might not work.\n"),
! pset.progname, client_ver / 10000, (client_ver / 100) % 100,
! pset.sversion / 10000, (pset.sversion / 100) % 100);
!
! printf(_("Type \"help\" for help.\n\n"));

#ifdef USE_SSL
printSSLInfo();


--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Alvaro Herrera wrote:
> Andrew Dunstan wrote:
>
>> Welcome to UI development. There is always *far* more argument of minor
>> matters of appearance than over anything else, in my experience.
>
> Which is a good thing (in this case at least), because otherwise we
> would end up with a crappy UI just because a single person thinks it's
> "good enough".


This makes me think we shouldn't be hard-coding anything at all
as the welcome message; but rather having a default .psqlrc
in much the same way that that there's a default /etc/bash.bashrc
and /etc/csh.login.

Within that default .psqlrc we can put
\qecho "Whatever the default message is"
or
select "my message "+version();
to create the default, but then anyone with their own .psqlrc
can re-define it to whatever they think is a "good enough" UI.


--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Ron Mayer wrote:
> Alvaro Herrera wrote:
> > Andrew Dunstan wrote:
> >
> >> Welcome to UI development. There is always *far* more argument of minor
> >> matters of appearance than over anything else, in my experience.
> >
> > Which is a good thing (in this case at least), because otherwise we
> > would end up with a crappy UI just because a single person thinks it's
> > "good enough".
>
>
> This makes me think we shouldn't be hard-coding anything at all
> as the welcome message; but rather having a default .psqlrc
> in much the same way that that there's a default /etc/bash.bashrc
> and /etc/csh.login.
>
> Within that default .psqlrc we can put
> \qecho "Whatever the default message is"
> or
> select "my message "+version();
> to create the default, but then anyone with their own .psqlrc
> can re-define it to whatever they think is a "good enough" UI.

We could do that but we still have to design the default banner.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Bruce Momjian <bruce@momjian.us> writes:
> Ron Mayer wrote:
>> This makes me think we shouldn't be hard-coding anything at all
>> as the welcome message; but rather having a default .psqlrc
>> in much the same way that that there's a default /etc/bash.bashrc
>> and /etc/csh.login.
>>
>> Within that default .psqlrc we can put
>> \qecho "Whatever the default message is"
>> or
>> select "my message "+version();
>> to create the default, but then anyone with their own .psqlrc
>> can re-define it to whatever they think is a "good enough" UI.

> We could do that but we still have to design the default banner.

More to the point, we would then have to design API with which a
custom .psqlrc could put out information about psql version,
server version, SSL status, etc. It would take a lot of work
to make this approach actually useful, and there isn't demand
to justify it AFAIK.

It's worth polishing the default behavior in any case, because
that is what newbies are going to see.

regards, tom lane

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

I'm OK with thisG but please move the printSSLInfo() call just before
echoing the help line.

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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches