From: David Fetter <david@fetter.org>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Jeff Janes <jeff.janes@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Making tab-complete.c easier to maintain
Date: Wed, 9 Dec 2015 06:27:09 -0800
Message-ID: <20151209142709.GA7050@fetter.org> (raw)
In-Reply-To: <CAB7nPqRoQVqzz9y5hOYBhMwPy5_OoTrObf=5a1XP+zjkPNAAkg@mail.gmail.com>
References: <CAEepm=1=Hdt9TCM3BRiVE6NCnXHyDDki+YVPhfZ02_=niU=8cA@mail.gmail.com>
<CAMkU=1yt=Ed7+K4sUisdUkt-wDhGnxdkDqs_43uJvhJYFA56gQ@mail.gmail.com>
<CAEepm=1ns0rbo+xun_j6_PwDT6zmEPuNggy=Vaie-3peF2C44Q@mail.gmail.com>
<CAMkU=1zRsaaSF_8qyQNYqOcDt0Xa7Awk==YdWCeTZtNhjnPNww@mail.gmail.com>
<CAEepm=1Oaq+L_6KCZ5o_hNn-=T93_LiK=_T1C9DcPzgXs4H84g@mail.gmail.com>
<CAEepm=1EOiP9_fKfb2B_ncp2hWYFxWEM+OsoD-=-iL_Wh=46Vg@mail.gmail.com>
<20151116151923.GX614468@alvherre.pgsql>
<CAB7nPqTfXV5Jwd1j-JF4wTuVgt8ggsNAy=yBByWB0nV1_3CEsA@mail.gmail.com>
<CAEepm=0+vDukbD7St2tjvpF9Ro9Z5Gj1Q9Mh6hKg+Of2YMNxzQ@mail.gmail.com>
<CAB7nPqRoQVqzz9y5hOYBhMwPy5_OoTrObf=5a1XP+zjkPNAAkg@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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.munro@enterprisedb.com> wrote:
> > On Mon, Dec 7, 2015 at 8:41 PM, Michael Paquier
> > <michael.paquier@gmail.com> wrote:
> >> On Tue, Nov 17, 2015 at 12:19 AM, Alvaro Herrera <alvherre@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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: david@fetter.org, michael.paquier@gmail.com, thomas.munro@enterprisedb.com, alvherre@2ndquadrant.com, jeff.janes@gmail.com, tgl@sss.pgh.pa.us
Subject: Re: Making tab-complete.c easier to maintain
In-Reply-To: <20151209142709.GA7050@fetter.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox