Thread: pgsql: Add new psql help topics, accessible to both --help and \?.

Add new psql help topics, accessible to both --help and \?.

Add --help=<topic> for the commandline, and \? <topic> as a backslash
command, to show more help than the invocations without parameters
do. "commands", "variables" and "options" currently exist as help
topics describing, respectively, backslash commands, psql variables,
and commandline switches. Without parameters the help commands show
their previous topic.

Some further wordsmithing or extending of the added help content might
be needed; but there seems little benefit delaying the overall feature
further.

Author: Pavel Stehule, editorialized by many

Reviewed-By: Andres Freund, Petr Jelinek, Fujii Masao, MauMau, Abhijit
    Menon-Sen and Erik Rijkers.

Discussion: CAFj8pRDVGuC-nXBfe2CK8vpyzd2Dsr9GVpbrATAnZO=2YQ0s2Q@mail.gmail.com,
    CAFj8pRA54AbTv2RXDTRxiAd8hy8wxmoVLqhJDRCwEnhdd7OUkw@mail.gmail.com

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/07c8651dd91d5aea38bee268acb582674ca4b5ea

Modified Files
--------------
doc/src/sgml/ref/psql-ref.sgml |   23 ++++-
src/bin/psql/command.c         |   14 ++-
src/bin/psql/help.c            |  210 +++++++++++++++++++++++++++++++---------
src/bin/psql/help.h            |    4 +-
src/bin/psql/startup.c         |   29 ++++--
src/bin/psql/tab-complete.c    |    7 ++
6 files changed, 224 insertions(+), 63 deletions(-)

On 2014-09-09 22:12:35 +0000, Andres Freund wrote:
> Add new psql help topics, accessible to both --help and \?.
>
> Add --help=<topic> for the commandline, and \? <topic> as a backslash
> command, to show more help than the invocations without parameters
> do. "commands", "variables" and "options" currently exist as help
> topics describing, respectively, backslash commands, psql variables,
> and commandline switches. Without parameters the help commands show
> their previous topic.
>
> Some further wordsmithing or extending of the added help content might
> be needed; but there seems little benefit delaying the overall feature
> further.
>
> Author: Pavel Stehule, editorialized by many
>
> Reviewed-By: Andres Freund, Petr Jelinek, Fujii Masao, MauMau, Abhijit
>     Menon-Sen and Erik Rijkers.
>
> Discussion: CAFj8pRDVGuC-nXBfe2CK8vpyzd2Dsr9GVpbrATAnZO=2YQ0s2Q@mail.gmail.com,
>     CAFj8pRA54AbTv2RXDTRxiAd8hy8wxmoVLqhJDRCwEnhdd7OUkw@mail.gmail.com

Hm. This fails on windows right now because the getopt_long() fallback
implementation doesn't implement optional_argument. Unless people think
this can be broken for a day or somebody has a better solution, I'll
revert tomorrow morning.
The best plan after that seems to be to add optional_argument support to
getopt_long.c - looks easy enough. Do we need a configure test for
optional_argument? I don't think so, but I could see somebody arguing
the other way round.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Wed, Sep 10, 2014 at 10:04 AM, Andres Freund <andres@anarazel.de> wrote:
> The best plan after that seems to be to add optional_argument support to
> getopt_long.c - looks easy enough. Do we need a configure test for
> optional_argument? I don't think so, but I could see somebody arguing
> the other way round.
Adding support for optional_argument in src/port would be the best
solution for long-term purposes. So +1 for it or we'll be limited in
the future as well. Here is for example attached a proof-of-concept
patch that it is not that complicated to add support for optional
arguments... Interestingly, the current implementation dates of 2003
in 939a59f and has not been majorly touched since. The patch has been
tested with MSVC.
--
Michael

Michael Paquier wrote:
> On Wed, Sep 10, 2014 at 10:04 AM, Andres Freund <andres@anarazel.de> wrote:
> > The best plan after that seems to be to add optional_argument support to
> > getopt_long.c - looks easy enough. Do we need a configure test for
> > optional_argument? I don't think so, but I could see somebody arguing
> > the other way round.
> Adding support for optional_argument in src/port would be the best
> solution for long-term purposes. So +1 for it or we'll be limited in
> the future as well. Here is for example attached a proof-of-concept
> patch that it is not that complicated to add support for optional
> arguments... Interestingly, the current implementation dates of 2003
> in 939a59f and has not been majorly touched since. The patch has been
> tested with MSVC.

FWIW we could have used this in the -X/-x mess in pg_basebackup .. but
probably that ship is far at sea.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Michael Paquier wrote:
> On Wed, Sep 10, 2014 at 10:04 AM, Andres Freund <andres@anarazel.de> wrote:
> > The best plan after that seems to be to add optional_argument support to
> > getopt_long.c - looks easy enough. Do we need a configure test for
> > optional_argument? I don't think so, but I could see somebody arguing
> > the other way round.
> Adding support for optional_argument in src/port would be the best
> solution for long-term purposes. So +1 for it or we'll be limited in
> the future as well. Here is for example attached a proof-of-concept
> patch that it is not that complicated to add support for optional
> arguments... Interestingly, the current implementation dates of 2003
> in 939a59f and has not been majorly touched since. The patch has been
> tested with MSVC.

Hmm, IIRC (didn't check) we only use our getopt_long if the platform
doesn't have one.  But what if the platform has one and it doesn't
support optional_argument?  I think we'd need additional configure black
magic to ensure that our version is chosen in these cases.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 2014-09-10 10:49:37 -0300, Alvaro Herrera wrote:
> Michael Paquier wrote:
> > On Wed, Sep 10, 2014 at 10:04 AM, Andres Freund <andres@anarazel.de> wrote:
> > > The best plan after that seems to be to add optional_argument support to
> > > getopt_long.c - looks easy enough. Do we need a configure test for
> > > optional_argument? I don't think so, but I could see somebody arguing
> > > the other way round.
> > Adding support for optional_argument in src/port would be the best
> > solution for long-term purposes. So +1 for it or we'll be limited in
> > the future as well. Here is for example attached a proof-of-concept
> > patch that it is not that complicated to add support for optional
> > arguments... Interestingly, the current implementation dates of 2003
> > in 939a59f and has not been majorly touched since. The patch has been
> > tested with MSVC.
>
> Hmm, IIRC (didn't check) we only use our getopt_long if the platform
> doesn't have one.  But what if the platform has one and it doesn't
> support optional_argument?  I think we'd need additional configure black
> magic to ensure that our version is chosen in these cases.

Yea, I was wondering about that too. But getopt_long() is a gnu
extension, and afaics had optional_argument since basically forever. A
quick search shows that it's been added to glibc before the historic
import into git (reaching back to 1995). So I think we don't need check
against it until we actually find some obscure situation where it
actually breaks.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 2014-09-10 20:15:31 +0900, Michael Paquier wrote:
> On Wed, Sep 10, 2014 at 10:04 AM, Andres Freund <andres@anarazel.de> wrote:
> > The best plan after that seems to be to add optional_argument support to
> > getopt_long.c - looks easy enough. Do we need a configure test for
> > optional_argument? I don't think so, but I could see somebody arguing
> > the other way round.
> Adding support for optional_argument in src/port would be the best
> solution for long-term purposes. So +1 for it or we'll be limited in
> the future as well. Here is for example attached a proof-of-concept
> patch that it is not that complicated to add support for optional
> arguments... Interestingly, the current implementation dates of 2003
> in 939a59f and has not been majorly touched since. The patch has been
> tested with MSVC.

I've attached a edited verion of that patch. Besides cosmetic stuff I
made one imo important change. Your version looked to the next argument
for optional arguments: When --help is an optional argument and e.g. the
commandline is "--help --host" we can't accept --host as --help's value.
So there's now a check for required_arg for that behaviour - which is in
line with all the getopt_long() behaviours I could find docs (or code in
case of netbsd) for.

I'd not mind a review, but I'll push this soon. Doesn't seem worth
reverting the entire psql patch when the fix is as localized as this. We
might conceivably want to change the behaviour of optional_argument, but
as there's only one caller at this point.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Thu, Sep 11, 2014 at 12:06 AM, Andres Freund <andres@anarazel.de> wrote:
> I've attached a edited version of that patch. Besides cosmetic stuff I
> made one imo important change. Your version looked to the next argument
> for optional arguments: When --help is an optional argument and e.g. the
> commandline is "--help --host" we can't accept --host as --help's value.
> So there's now a check for required_arg for that behaviour - which is in
> line with all the getopt_long() behaviours I could find docs (or code in
> case of netbsd) for.
Thanks for spotting this. Yes I clearly missed that path.

> I'd not mind a review, but I'll push this soon. Doesn't seem worth
> reverting the entire psql patch when the fix is as localized as this. We
> might conceivably want to change the behaviour of optional_argument, but
> as there's only one caller at this point.
Thanks. Even if we are not on the same timezone, you could have waited
a bit for some feedback from someone with Windows environment IMO. It
is always good to have an insurance.

Regards,
--
Michael

On 2014-09-11 09:52:20 +0900, Michael Paquier wrote:
> > I'd not mind a review, but I'll push this soon. Doesn't seem worth
> > reverting the entire psql patch when the fix is as localized as this. We
> > might conceivably want to change the behaviour of optional_argument, but
> > as there's only one caller at this point.
> Thanks. Even if we are not on the same timezone, you could have waited
> a bit for some feedback from someone with Windows environment IMO. It
> is always good to have an insurance.

The reason that I pushed it quickly was that windows on the buildfarm
already was broken. This wouldn't (and afaics hasn't) make it any worse.
The longer members stay read, the more changes can be in that broken
window. Thus it gets harder to pinpoint problems. It's also not like any
code in getopt_long.c is particularly windows specific - after including
in Makefile.global it can easily be run on linux.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services