On Mon, Feb 15, 2021 at 03:57:04PM +0900, Ian Lawrence Barwick wrote:
> Indeed it does. Not the most exciting of use cases, though I imagine it
> might come in handy for anyone developing an extension, and the
> existing implementation is inconsistent (in place for ALTER INDEX,
> and partially for ALTER MATERIALIZED VIEW, but not the others).
> Patch suggestion attached.
Thanks.
- else if (Matches("ALTER", "INDEX", MatchAny, "NO", "DEPENDS"))
- COMPLETE_WITH("ON EXTENSION");
- else if (Matches("ALTER", "INDEX", MatchAny, "DEPENDS"))
- COMPLETE_WITH("ON EXTENSION");
The part, if removed, means that typing "alter index my_index no " is
not able to complete with "DEPENDS ON EXTENSION" anymore. So it seems
to me that ALTER INDEX got that right, and that the other commands had
better do the same.
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.
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?).
Regards
Ian Barwick
--
