Re: mprove tab completion for ALTER EXTENSION ADD/DROP
On Mon, 5 Dec 2022 at 06:53, Michael Paquier wrote: > > On Sat, Dec 03, 2022 at 05:34:57PM +, 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 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 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 From c2aeeadac88b709ce823a01c80b0b8455c62d3d9 Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Mon, 2 Jan 2023 08:01:45 +0530 Subject: [PATCH v2 2/2] Missing tab completion for ALTER EXTENSION DROP Missing tab completion for ALTER EXTENSION DROP --- src/bin/psql/tab-complete.c | 25 ++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 4d1a664bf7..a6f27ed9b8 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -224,6 +224,7 @@ static char *completion_ref_schema; /* schema name of reference object */ static bool completion_case_sensitive; /* completion is case sensitive */ static bool completion_verbatim; /* completion is verbatim */ static bool completion_force_quote; /* true to force-quote filenames */ +static bool completion_dont_quote; /* true to not quote the result */ /* * A few macros to ease typing. You can use these to complete the given @@ -1011,6 +1012,15 @@ static const SchemaQuery Query_for_trigger_of_table = { "SELECT nspname FROM pg_catalog.pg_namespace "\ " WHERE nspname LIKE '%s'" +#define Query_for_extension_objs \ +"SELECT distinct CASE WHEN p.type = 'foreign-data wrapper' "\ +" THEN 'FOREIGN DATA WRAPPER'"\ +" ELSE trim ('\"' from pg_catalog.upper(p.type::text)) END "\ +" FROM pg_depend, pg_identify_object(classid, objid, objsubid) AS p"\ +" WHERE refclassid = 'pg_extension'::regclass AND"\ +" pg_catalog.lower(p.type) LIKE pg_catalog.lower('%s') AND"\ +" refobjid = (SELECT oid FROM pg_extension WHERE extname = '%s')" + /* Use COMPLETE_WITH_QUERY_VERBATIM with these queries for GUC names: */ #define Query_for_list_of_alter_system_set_vars \ "SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\ @@ -1946,6 +1956,15 @@ psql_completion(const char *text, int start, int end) "ROUTINE", "SCHEMA", "SEQUENCE", "SERVER", "TABLE", "TEXT SEARCH", "TRANSFORM FOR", "TYPE", "VIEW"); + /* ALTER EXTENSION DROP */ + else if (Matches("ALTER", "EXTENSION", MatchAny, "DROP")) + { + set_completion_reference(prev2_wd); + completion_dont_quote = true; + COMPLETE_WITH_QUERY(Query_for_extension_objs); + completion_dont_quote = false; + } + /* ALTER EXTENSION ADD FOREIGN*/ else if (Matches("ALTER", "EXTENSION", MatchAny, "ADD", "FOREIGN")) COMPLETE_WITH("DATA WRAPPER", "TABLE"); @@ -5313,7 +5332,7 @@ _complete_from_query(const char *simple_query, * surprising. This restriction also dodges some odd behaviors of * some versions of readline/libedit. */ - if (non_empty_object) + if (non_empty_object && !completion_dont_quote) { if (item && !objectquoted && identifier_needs_quotes(item)) continue; @@ -5910,7 +5929,7 @@ requote_identifier(const char *schemaname, const char *objectname, if (schemaname) { buflen += strlen(schemaname) + 1; /* +1 for the dot */ - if (!quote_schema) + if (!quote_schema && !completion_dont_quote) quote_schema = identifier_needs_quotes(schemaname); if (quote_schema) { @@ -5925,7 +5944,7 @@ requote_identifier(const char *schemaname, const char *objectname, if (objectname) { buflen += strlen(objectname); - if (!quote_object) + if (!quote_object && !completion_dont_quote) quote_object = identifier_needs_quotes(objectname); if (quote_object) { -- 2.34.1 From 328509d60c1ec5489f5e29efbabc4c32466d1bac Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Sat, 26 Nov 2022 20
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
At Mon, 2 Jan 2023 13:19:50 +0530, vignesh C wrote in > On Mon, 5 Dec 2022 at 06:53, Michael Paquier 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
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
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 signature.asc Description: PGP signature
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
On Wed, 11 Jan 2023 at 12:19, Michael Paquier 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
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
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 signature.asc Description: PGP signature
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
On Thu, 12 Jan 2023 at 05:22, Michael Paquier 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
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
--- Original Message --- On Sunday, November 27th, 2022 at 10:24, vignesh C 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
Re: mprove tab completion for ALTER EXTENSION ADD/DROP
On Sat, Dec 03, 2022 at 05:34:57PM +, 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 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 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 signature.asc Description: PGP signature