On Wed, Dec 09, 2015 at 08:49:22PM +0900, Michael Paquier wrote: > On Wed, Dec 9, 2015 at 8:17 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: > > On Mon, Dec 7, 2015 at 8:41 PM, Michael Paquier > > <michael.paqu...@gmail.com> wrote: > >> On Tue, Nov 17, 2015 at 12:19 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> > >> wrote: > >>> Thomas Munro wrote: > >>>> New version attached, merging recent changes. > >>> > >>> I wonder about the TailMatches and Matches macros --- wouldn't it be > >>> better to have a single one, renaming TailMatches to Matches and > >>> replacing the current Matches() with an initial token that corresponds > >>> to anchoring to start of command? Just wondering, not terribly attached > >>> to the idea. > >> > >> + /* TODO:TM -- begin temporary, not part of the patch! */ > >> + Assert(!word_matches(NULL, "")); > >> + [...] > >> + Assert(!word_matches("foo", "")); > >> + /* TODO:TM -- end temporary */ > >> > >> Be sure to not forget to remove that later. > > > > Thanks for looking at this Michael. It's probably not much fun to > > review! Here is a new version with that bit removed. More responses > > inline below. > > I had a hard time not sleeping when reading it... That's very mechanical. > > > I agree that would probably be better but Alvaro suggested following > > the existing logic in the first pass, which was mostly based on tails, > > and then considering simpler head-based patterns in a future pass. > > Fine with me. > > So what do we do now? There is your patch, which is already quite big, > but as well a second patch based on regexps, which is far bigger. And > at the end they provide a similar result: > > Here is for example what the regexp patch does for some complex > checks, like ALTER TABLE RENAME: > /* ALTER TABLE xxx RENAME yyy */ > - else if (pg_strcasecmp(prev4_wd, "TABLE") == 0 && > - pg_strcasecmp(prev2_wd, "RENAME") == 0 && > - pg_strcasecmp(prev_wd, "CONSTRAINT") != 0 && > - pg_strcasecmp(prev_wd, "TO") != 0) > + else if (MATCH("TABLE #id RENAME !CONSTRAINT|TO")) > > And what Thomas's patch does: > + else if (TailMatches5("ALTER", "TABLE", MatchAny, "RENAME", MatchAny) && > + !TailMatches1("CONSTRAINT|TO")) > > The regexp patch makes the negative checks somewhat easier to read > (there are 19 positions in tab-complete.c doing that), still inventing > a new langage and having a heavy refactoring just tab completion of > psql seems a bit too much IMO, so my heart balances in favor of > Thomas' stuff. Thoughts from others?
Agreed that the "whole new language" aspect seems like way too big a hammer, given what it actually does. Cheers, David. -- David Fetter <da...@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers