On Thu, Jan 28, 2016 at 10:15 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Thu, Jan 28, 2016 at 9:32 PM, Fujii Masao <masao.fu...@gmail.com> wrote:
>> I found that the following tab-completions for SET/RESET which
>> worked properly before doesn't work properly now in the master.
>>
>> 1. ALTER SYSTEM SET|RESET <tab> lists nothing.
>> 2. ALTER DATABASE xxx SET <tab> lists nothing.
>> 3. ALTER DATABASE xxx SET yyy <tab> lists nothing.
>> 4. ALTER DATABASE xxx SET datestyle TO <tab> lists nothing.
>>
>> Attached patch fixes those problems.
>
> -       else if (Matches4("ALTER", "SYSTEM", "SET|RESET", MatchAny))
> +       else if (Matches3("ALTER", "SYSTEM", "SET|RESET"))
> Good catch.
>
> -       else if (Matches2("SET", MatchAny))
> +       else if (TailMatches2("SET", MatchAny) &&
> +                        !TailMatches4("UPDATE|DOMAIN", MatchAny,
> MatchAny, MatchAny) &&
> +                        !TailMatches1("TABLESPACE|SCHEMA") &&
> +                        !ends_with(prev_wd, ')') &&
> +                        !ends_with(prev_wd, '='))
>                 COMPLETE_WITH_CONST("TO");
> This gets... unreadable.
>
> In order to maximize the amount of Matches() used, wouldn't it be
> better to complete a bit more the list of completions directly in
> ALTER DATABASE? This would make the code more readable.

Thanks for the review!

I removed the above and added the following for that case.

+    /* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET <name> */
+    else if (Matches2("ALTER", "DATABASE|FUNCTION|ROLE|USER") &&
+             TailMatches2("SET", MatchAny))
+        COMPLETE_WITH_LIST2("FROM CURRENT", "TO");

Attached is the updated version of the patch.
I also added the change that Sawada suggested, to the patch.

Regards,

-- 
Fujii Masao
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1553,1559 **** psql_completion(const char *text, int start, int end)
  	else if (Matches2("ALTER", "SYSTEM"))
  		COMPLETE_WITH_LIST2("SET", "RESET");
  	/* ALTER SYSTEM SET|RESET <name> */
! 	else if (Matches4("ALTER", "SYSTEM", "SET|RESET", MatchAny))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW <name> */
  	else if (Matches3("ALTER", "VIEW", MatchAny))
--- 1553,1559 ----
  	else if (Matches2("ALTER", "SYSTEM"))
  		COMPLETE_WITH_LIST2("SET", "RESET");
  	/* ALTER SYSTEM SET|RESET <name> */
! 	else if (Matches3("ALTER", "SYSTEM", "SET|RESET"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW <name> */
  	else if (Matches3("ALTER", "VIEW", MatchAny))
***************
*** 1754,1759 **** psql_completion(const char *text, int start, int end)
--- 1754,1762 ----
  	 */
  	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "TABLESPACE"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_tablespaces);
+ 	/* If we have ALTER TABLE <sth> SET WITH provide OIDS */
+ 	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "WITH"))
+ 		COMPLETE_WITH_CONST("OIDS");
  	/* If we have ALTER TABLE <sth> SET WITHOUT provide CLUSTER or OIDS */
  	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "WITHOUT"))
  		COMPLETE_WITH_LIST2("CLUSTER", "OIDS");
***************
*** 2702,2708 **** psql_completion(const char *text, int start, int end)
  
  /* SET, RESET, SHOW */
  	/* Complete with a variable name */
! 	else if (Matches1("SET|RESET"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
  	else if (Matches1("SHOW"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_show_vars);
--- 2705,2711 ----
  
  /* SET, RESET, SHOW */
  	/* Complete with a variable name */
! 	else if (TailMatches1("SET|RESET") && !TailMatches3("UPDATE", MatchAny, "SET"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
  	else if (Matches1("SHOW"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_show_vars);
***************
*** 2743,2750 **** psql_completion(const char *text, int start, int end)
  	/* Complete SET <var> with "TO" */
  	else if (Matches2("SET", MatchAny))
  		COMPLETE_WITH_CONST("TO");
  	/* Suggest possible variable values */
! 	else if (Matches3("SET", MatchAny, "TO|="))
  	{
  		/* special cased code for individual GUCs */
  		if (TailMatches2("DateStyle", "TO|="))
--- 2746,2757 ----
  	/* Complete SET <var> with "TO" */
  	else if (Matches2("SET", MatchAny))
  		COMPLETE_WITH_CONST("TO");
+ 	/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET <name> */
+ 	else if (Matches2("ALTER", "DATABASE|FUNCTION|ROLE|USER") &&
+ 			 TailMatches2("SET", MatchAny))
+ 		COMPLETE_WITH_LIST2("FROM CURRENT", "TO");
  	/* Suggest possible variable values */
! 	else if (TailMatches3("SET", MatchAny, "TO|="))
  	{
  		/* special cased code for individual GUCs */
  		if (TailMatches2("DateStyle", "TO|="))
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to