Date: Mon, 7 Sep 2015 10:56:05 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Making tab-complete.c easier to maintain
Message-ID: <20150907135605.GP2912@alvherre.pgsql>
References: 
 <CAEepm=2wF0eTXfQUMzvJ71GC47ESQmWQ3Y_fz5CRBask1uA1DA@mail.gmail.com>
 <17081.1441374043@sss.pgh.pa.us>
 <CAEepm=0QKjnVVq9ueBXP5yntNFwtFZhFzLPEOpCA-PPoh0NieQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAEepm=0QKjnVVq9ueBXP5yntNFwtFZhFzLPEOpCA-PPoh0NieQ@mail.gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Thomas Munro wrote:

> Thanks, good point.  Here's a version that uses NULL via a macro ANY.
> Aside from a few corrections it also now distinguishes between
> TAIL_MATCHESn (common) and MATCHESn (rarely used for now), for example:

This looks pretty neat -- 100x neater than what we have, at any rate.  I
would use your new MATCHESn() macros a bit more -- for instance the
completion for "ALTER but not ALTER after ALTER TABLE" could be
rephrased as simply MATCHES1("ALTER"), i.e. have it match at start of
command only.  Maybe that's just a matter of going over the new code
after the initial run, so that we can have a first patch that's mostly
mechanical and a second pass in which more semantically relevant changes
are applied.  Seems easier to review ...

I would use "ANY" as a keyword here.  Sounds way too generic to me.
Maybe "CompleteAny" or something like that.

Stylistically, I find there's too much uppercasing here.  Maybe rename the
macros like this instead:

> +       else if (TailMatches4("ALL", "IN", "TABLESPACE", ANY))
> +               CompleteWithList2("SET TABLESPACE", "OWNED BY");

Not totally sure about this part TBH.

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


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