Re: Making tab-complete.c easier to maintain

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: alvherre(at)2ndquadrant(dot)com
Cc: jeff(dot)janes(at)gmail(dot)com, thomas(dot)munro(at)enterprisedb(dot)com, tgl(at)sss(dot)pgh(dot)pa(dot)us, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Making tab-complete.c easier to maintain
Date: 2015-11-12 04:16:27
Message-ID: 20151112.131627.11088278.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello. How about regular expressions?

I've been thinking of better mechanism for tab-compltion for
these days since I found some bugs in it.

At Fri, 23 Oct 2015 14:50:58 -0300, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote in <20151023175058(dot)GA3391(at)alvherre(dot)pgsql>
> Jeff Janes wrote:
>
> > For the bigger picture, I don't think we should not apply this patch simply
> > because there is something even better we might theoretically do at some
> > point in the future.
>
> Agreed.

Auto-generating from grammer should be the ultimate solution but
I don't think it will be available. But still I found that the
word-splitting-then-match-word-by-word-for-each-matching is
terriblly unmaintainable and poorly capable. So, how about
regular expressions?

I tried to use pg_regex in frontend and found that it is easily
doable. As a proof of the concept, the two patches attached to
this message does that changes.

1. 0001-Allow-regex-module-to-be-used-outside-server.patch

This small change makes pg_regex possible to be used in
frontend.

2. 0002-Replace-previous-matching-rule-with-regexps.patch

Simply replaces existing matching rules almost one-by-one with
regular expression matches.

I made these patches not to change the behavior except inevitable
ones.

We would have far powerful matching capability using regular
expressions and it makes tab-complete.c look simpler. On the
other hand, regular expressions - which are stashed away into new
file by this patch - is a chunk of complexity and (also) error
prone. For all that I think this is better than the current
situation in terms of maintainability and capability.

This should look stupid because it really be replaced stupidly
and of course this can be more sane/effective/maintainable by
refactoring. But before that issue, I'm not confident at all that
this is really a alternative with *gigantic* improvement.

Any opinions?

> > Having used it a little bit, I do agree with Robert
> > that it is not a gigantic improvement over the current situation, as the
> > code it replaces is largely mechanical boilerplate. But I think it is
> > enough of an improvement that we should go ahead with it.
>
> To me this patch sounds much like 2eafcf68d563df8a1db80a. You could say
> that what was replaced was "largely mechanical", but it was so much
> easier to make mistakes with the original coding that it's not funny.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-Allow-regex-module-to-be-used-outside-server.patch text/x-patch 4.3 KB
0002-Replace-previous-matching-rule-with-regexps.patch text/x-patch 190.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kouhei Kaigai 2015-11-12 04:34:06 Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)
Previous Message Kouhei Kaigai 2015-11-12 04:13:37 Re: CustomScan in a larger structure (RE: CustomScan support on readfuncs.c)