Re: psql: tab completion differs on semicolon placement - Mailing list pgsql-hackers

From David Fetter
Subject Re: psql: tab completion differs on semicolon placement
Date
Msg-id 20210920230401.GL18391@fetter.org
Whole thread Raw
In response to Re: psql: tab completion differs on semicolon placement  (Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>)
Responses Re: psql: tab completion differs on semicolon placement
List pgsql-hackers
On Mon, Sep 20, 2021 at 08:26:51PM +0100, Dagfinn Ilmari Mannsåker wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:
> 
> > While testing a patch I fat-fingered a CREATE DATABASE statement by tab
> > completing *after* the semicolon, with no space between the objname and
> > semicolon.  The below options were presented, which at this point aren't really
> > applicable:
> >
> > db=# create database foo;
> > ALLOW_CONNECTIONS ENCODING          LC_COLLATE        LOCALE            TABLESPACE
> > CONNECTION LIMIT  IS_TEMPLATE       LC_CTYPE          OWNER             TEMPLATE
> >
> > DROP DATABASE has a similar tab completion which makes about as much sense:
> >
> > db=# drop database foo;WITH (
> >
> > Checking prev_wd for not ending with ';' as per the attached makes "objname;"
> > behave like "objname ;".  Is there a reason for not doing that which I'm
> > missing?  I didn't check for others, but if this seems reasonable I'll go
> > through to find any other similar cases.
> 
> The same applies to any completion after a MatchAny that ends in a any
> of the WORD_BREAKS characters (except whitespace and () which are
> handled specially).
> 
> #define WORD_BREAKS             "\t\n@$><=;|&{() "
> 
> IMO a fix should be more principled than just special-casing semicolon
> and CREATE TABLE.  Maybe get_previous_words() should stop when it sees
> an unquoted semicolon?

Is there some reason get_previous_words() shouldn't stop for
everything that's WORD_BREAKS? If not, making that the test might make the
general rule a little simpler to write, and if WORD_BREAKS ever
changed, for example to include all space, or all breaking space, or
similar, the consequences would at least not propagate through
seemingly unrelated code.

At the moment, get_previous_words() does look for everything in
WORD_BREAKS, and then accounts for double quotes (") and then does
something clever to account for double quotes and the quoting behavior
that doubling them ("") accomplishes. Anyhow, that looks like it
should work in this case, but clearly it's not.

Would it be less error prone to do these checks and maybe push or pop
one or more stacks holding state as each character came in? I suspect
the overhead would be unnoticeable even on the slowest* client.

Best,
David.

* One possible exception would be a gigantic paste, a case where psql
  can be prevented from attempting tab completion, although the
  prevention measures involve a pretty obscure terminal setting:
  https://cirw.in/blog/bracketed-paste

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate



pgsql-hackers by date:

Previous
From: "Bossart, Nathan"
Date:
Subject: Re: .ready and .done files considered harmful
Next
From: Alexander Korotkov
Date:
Subject: Re: postgres.h included from relcache.h - but removing it breaks pg_upgrade