On Tue, Feb 16, 2021 at 11:18:47AM +0900, Ian Lawrence Barwick wrote:
> Hmm, with the current implementation "alter index my_index no <TAB>"
> doesn't work
> anyway; you'd need to add this before the above lines:
>
> + else if (Matches("ALTER", "INDEX", MatchAny, "NO"))
> + COMPLETE_WITH("DEPENDS");
>
> so AFAICT the patch doesn't change that behaviour. It does mean "alter index
> my_index no depends <TAB>" no longer completes to "ON EXTENSION", but if
> you've
> typed one of "NO" or "DEPENDS" in that context, "ON EXTENSION" is the only
> completion so I'm not sure what's gained by forcing the user to hit TAB
> twice.
You are right. It looks like I have tested without a whitespace after
the "NO". With a whitespace it does not work, so that looks like a
complication for little gain. Another problem with the code on HEAD
is that you would not complete properly "NO DEPENDS ON", so that feels
half-completed.
> There are quite a few tab completions consisting of more than one word
> (e.g. "MATERIALIZED VIEW", "FORCE ROW LEVEL SECURITY") where tab completion
> is
> ineffective after the first word followed by a space, e.g. "alter
> materialized
> <TAB>" doesn't result in any expansion either. I suppose we could go
> through all
> those and handle each word individually, but presumably there's a reason why
> that hasn't been done already (maybe no-one has complained?).
Because that's just extra maintenance as most people will just
complete after typing the first set of characters? This part got
discussed as of 1e324cb:
https://www.postgresql.org/message-id/CALtqXTcogrFEVP9uou5vFtnGsn+vHZUu9+9a0inarfYVOHScYQ@mail.gmail.com
Anyway, after sleeping on it, I have just applied your original patch
as that's simpler, and will cover the cases people would care for.
--
Michael