Thread: mprove tab completion for ALTER EXTENSION ADD/DROP
Hi, Tab completion for ALTER EXTENSION ADD and DROP was missing, this patch adds the tab completion for the same. Regards, Vignesh
Attachment
------- 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
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
Attachment
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
Attachment
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
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
Attachment
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
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
Attachment
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