Horton

Thread

Commits

  1. Add support for tab completion after ALTER EXTENSION ADD|DROP in psql

  1. mprove tab completion for ALTER EXTENSION ADD/DROP

    vignesh C <vignesh21@gmail.com> — 2022-11-27T13:24:27Z

    Hi,

Tab completion for ALTER EXTENSION ADD and DROP was missing, this
patch adds the tab completion for the same.

Regards,
Vignesh

  2. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    Matheus Alcantara <mths.dev@pm.me> — 2022-12-03T17:34:57Z

    ------- Original Message -------
On Sunday, November 27th, 2022 at 10:24, vignesh C <vignesh21@gmail.com> wrote:


> Hi,
> 
> Tab completion for ALTER EXTENSION ADD and DROP was missing, this
> patch adds the tab completion for the same.
> 
> Regards,
> Vignesh

Hi Vignesh

I've tested your patched on current master and seems to be working properly.


I'm starting reviewing some patches here, let's see what more experience hackers
has to say about this, but as far I can tell is that is working as expected.


--
Matheus Alcantara

  3. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    Michael Paquier <michael@paquier.xyz> — 2022-12-05T01:23:09Z

    On Sat, Dec 03, 2022 at 05:34:57PM +0000, Matheus Alcantara wrote:
> I've tested your patched on current master and seems to be working properly.
> 
> I'm starting reviewing some patches here, let's see what more experience hackers
> has to say about this, but as far I can tell is that is working as expected.

+       /* ALTER EXTENSION <name> ADD|DROP */
+       else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD|DROP"))
+               COMPLETE_WITH("ACCESS METHOD", "AGGREGATE", "CAST", "COLLATION",
+                                         "CONVERSION", "DOMAIN", "EVENT TRIGGER", "FOREIGN",
+                                         "FUNCTION", "MATERIALIZED VIEW", "OPERATOR",
+                                         "PROCEDURAL LANGUAGE", "PROCEDURE", "LANGUAGE",
+                                         "ROUTINE", "SCHEMA", "SEQUENCE", "SERVER", "TABLE",
+                                         "TEXT SEARCH", "TRANSFORM FOR", "TYPE", "VIEW");
+
+       /* ALTER EXTENSION <name> ADD|DROP FOREIGN*/
+       else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD|DROP", "FOREIGN"))
+               COMPLETE_WITH("DATA WRAPPER", "TABLE");

The DROP could be matched with the objects that are actually part of
the so-said extension?
--
Michael

  4. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    vignesh C <vignesh21@gmail.com> — 2023-01-02T07:49:50Z

    On Mon, 5 Dec 2022 at 06:53, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, Dec 03, 2022 at 05:34:57PM +0000, Matheus Alcantara wrote:
> > I've tested your patched on current master and seems to be working properly.
> >
> > I'm starting reviewing some patches here, let's see what more experience hackers
> > has to say about this, but as far I can tell is that is working as expected.
>
> +       /* ALTER EXTENSION <name> ADD|DROP */
> +       else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD|DROP"))
> +               COMPLETE_WITH("ACCESS METHOD", "AGGREGATE", "CAST", "COLLATION",
> +                                         "CONVERSION", "DOMAIN", "EVENT TRIGGER", "FOREIGN",
> +                                         "FUNCTION", "MATERIALIZED VIEW", "OPERATOR",
> +                                         "PROCEDURAL LANGUAGE", "PROCEDURE", "LANGUAGE",
> +                                         "ROUTINE", "SCHEMA", "SEQUENCE", "SERVER", "TABLE",
> +                                         "TEXT SEARCH", "TRANSFORM FOR", "TYPE", "VIEW");
> +
> +       /* ALTER EXTENSION <name> ADD|DROP FOREIGN*/
> +       else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD|DROP", "FOREIGN"))
> +               COMPLETE_WITH("DATA WRAPPER", "TABLE");
>
> The DROP could be matched with the objects that are actually part of
> the so-said extension?

The modified v2 version has the changes to handle the same. Sorry for
the delay as I was working on another project.

Regards,
Vignesh

  5. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2023-01-11T03:10:33Z

    At Mon, 2 Jan 2023 13:19:50 +0530, vignesh C <vignesh21@gmail.com> wrote in 
> On Mon, 5 Dec 2022 at 06:53, Michael Paquier <michael@paquier.xyz> wrote:
> >
> > The DROP could be matched with the objects that are actually part of
> > the so-said extension?
> 
> The modified v2 version has the changes to handle the same. Sorry for
> the delay as I was working on another project.

It suggests the *kinds* of objects that are part of the extension, but
lists the objects of that kind regardless of dependency.  I read
Michael suggested (and I agree) to restrict the objects (not kinds) to
actually be a part of the extension. (And not for object kinds.)

However I'm not sure it is useful to restrict object kinds since the
operator already knows what to drop, if you still want to do that, the
use of completion_dont_quote looks ugly since the function
(requote_identifier) is assuming an identifier as input.  I didn't
looked closer but maybe we need another way to do that.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  6. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    Michael Paquier <michael@paquier.xyz> — 2023-01-11T06:49:44Z

    On Wed, Jan 11, 2023 at 12:10:33PM +0900, Kyotaro Horiguchi wrote:
> It suggests the *kinds* of objects that are part of the extension, but
> lists the objects of that kind regardless of dependency.  I read
> Michael suggested (and I agree) to restrict the objects (not kinds) to
> actually be a part of the extension. (And not for object kinds.)

Yeah, that's what I meant.  Now, if Vignesh does not want to extend
that, that's fine for me as well at the end on second thought, as this 
involves much more code for each DROP path depending on the object
type involved.

Adding the object names after DROP/ADD is useful on its own, and we
already have some completion once the object type is specified, so
simpler is perhaps just better here.
--
Michael

  7. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    vignesh C <vignesh21@gmail.com> — 2023-01-11T16:59:25Z

    On Wed, 11 Jan 2023 at 12:19, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jan 11, 2023 at 12:10:33PM +0900, Kyotaro Horiguchi wrote:
> > It suggests the *kinds* of objects that are part of the extension, but
> > lists the objects of that kind regardless of dependency.  I read
> > Michael suggested (and I agree) to restrict the objects (not kinds) to
> > actually be a part of the extension. (And not for object kinds.)
>
> Yeah, that's what I meant.  Now, if Vignesh does not want to extend
> that, that's fine for me as well at the end on second thought, as this
> involves much more code for each DROP path depending on the object
> type involved.
>
> Adding the object names after DROP/ADD is useful on its own, and we
> already have some completion once the object type is specified, so
> simpler is perhaps just better here.

I too felt keeping it simpler is better. How about using the simple
first version of patch itself?

Regards,
Vignesh

  8. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    Michael Paquier <michael@paquier.xyz> — 2023-01-11T23:52:13Z

    On Wed, Jan 11, 2023 at 10:29:25PM +0530, vignesh C wrote:
> I too felt keeping it simpler is better. How about using the simple
> first version of patch itself?

Okay, I have just done that, then, after checking that all the object
types were covered (28).  Note that PROCEDURAL LANGUAGE has been
removed for simplicity.
--
Michael

  9. Re: mprove tab completion for ALTER EXTENSION ADD/DROP

    vignesh C <vignesh21@gmail.com> — 2023-01-13T00:47:59Z

    On Thu, 12 Jan 2023 at 05:22, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jan 11, 2023 at 10:29:25PM +0530, vignesh C wrote:
> > I too felt keeping it simpler is better. How about using the simple
> > first version of patch itself?
>
> Okay, I have just done that, then, after checking that all the object
> types were covered (28).  Note that PROCEDURAL LANGUAGE has been
> removed for simplicity.

Thanks for pushing this.

Regards,
Vignesh