Re: Making tab-complete.c easier to maintain

On Thu, Dec 17, 2015 at 7:24 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Dec 17, 2015 at 3:06 PM, Kyotaro HORIGUCHI
> <horiguchi.kyotaro@lab.ntt.co.jp> wrote:
>> At Mon, 14 Dec 2015 14:13:02 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in
<CAB7nPqSggjPA8U1WV7ivW44xzboC8pci_Etmffr+ZEzxSX_ahA@mail.gmail.com>
>>> On Mon, Dec 14, 2015 at 8:10 AM, Thomas Munro <thomas.munro@enterprisedb.com> wrote:
>>> Yeah, I guess that's an improvement for those cases, and there is no
>>> immediate need for a per-keyword NOT operator in our cases to allow
>>> things of the type (foo1 OR NOT foo2). Still, in the case of this
>>> patch !foo1|foo2 actually means (NOT foo1 AND NOT foo2). It does not
>>> seem that much intuitive. Reading straight this expression it seems
>>> that !foo1|foo2 means actually (NOT foo1 OR foo2) because the lack of
>>> parenthesis. Thoughts?
>>
>> I used just the same expression as Thomas in my patch since it
>> was enough intuitive in this context in my view. The expression
>> "(!FOO1)|FOO2" is a nonsence in the context of tab-completion and
>> won't be used in future. But it is true that "!(FOO|BAR|BAZ)" is
>> clearer than without the parenthesis.
>
> Yeah that's my whole point. If we decide to support that I think that
> we had better go all the way through it, with stuff like:
> - & for AND operator
> - Support of parenthesis to prioritize expressions
> - ! for NOT operator
> - | for OR OPERATOR
> But honestly at this stage this would just benefit 5 places (citing
> Thomas' quote), hence let's just go to the most simple approach which
> is the one without all this fancy operator stuff... That will be less
> bug prone, and still benefits more than 95% of the tab completion code
> path. At least I think that's the most realistic thing to do if we
> want to get something for this commit fest. If someone wants those
> operators, well I guess that he could add them afterwards.. Thomas,
> what do you think?

I agree with both of you that using "!" without parentheses is not
ideal.  I also don't think it's worth trying to make a clever language
here: in future it will be a worthy and difficult project to do
something far cleverer based on the productions of the real grammar as
several people have said.  (Presumably with some extra annotations to
enable/disable productions and mark out points where database object
names are looked up?).  In the meantime, I think we should just do the
simplest thing that will replace the repetitive strcasecmp-based code
with something readable/writable, and avoid the
fully-general-pattern-language rabbit hole.

Kyotaro's suggestion of using a macro NEG x to avoid complicating the
string constants seems good to me.  But perhaps like this?

  TailMatches4("COMMENT", "ON", MatchAny, MatchAnyExcept("IS"))

See attached patch which does it that way.

>> Addition to that, I feel that successive "MatchAny"s are a bit
>> bothersome.
>>
>>>  TailMatches6("COMMENT", "ON", MatchAny, MatchAny, MatchAny, MatchAny)) &&
>>>              !TailMatches1("IS")
>>
>> Is MachAny<n> acceptable? On concern is the two n's
>> (TailMatches<n> and MatchAny<n>) looks a bit confising.
>>
>>>  TailMatches4("COMMENT", "ON", MatchAny3, "!IS")
>
> Ah, OK, so you would want to be able to have an inner list, MatchAnyN
> meaning actually a list of MatchAny items repeated N times. I am not
> sure if that's worth it.. I would just keep it simple, and we are just
> discussing about a couple of places only that would benefit from that.

+1 for simple.

--
Thomas Munro
http://www.enterprisedb.com