Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-02-01 Thread Michael Paquier
On Mon, Feb 1, 2016 at 10:22 PM, Fujii Masao  wrote:

> Pushed. Thanks!
>

OK. And attached is the promised patch for ALTER FUNCTION.
-- 
Michael
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 5f27120..3369a3d 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1370,7 +1370,7 @@ psql_completion(const char *text, int start, int end)
 	else if (Matches3("ALTER", "AGGREGATE|FUNCTION", MatchAny))
 		COMPLETE_WITH_CONST("(");
 	/* ALTER AGGREGATE,FUNCTION  (...) */
-	else if (Matches4("ALTER", "AGGREGATE|FUNCTION", MatchAny, MatchAny))
+	else if (Matches4("ALTER", "AGGREGATE", MatchAny, MatchAny))
 	{
 		if (ends_with(prev_wd, ')'))
 			COMPLETE_WITH_LIST3("OWNER TO", "RENAME TO", "SET SCHEMA");
@@ -1378,6 +1378,18 @@ psql_completion(const char *text, int start, int end)
 			COMPLETE_WITH_FUNCTION_ARG(prev2_wd);
 	}
 
+	/* ALTER FUNCTION  (...) */
+	else if (Matches4("ALTER", "FUNCTION", MatchAny, MatchAny))
+	{
+		if (ends_with(prev_wd, ')'))
+			COMPLETE_WITH_LIST4("OWNER TO", "RENAME TO", "SET", "RESET");
+		else
+			COMPLETE_WITH_FUNCTION_ARG(prev2_wd);
+	}
+	/* ALTER FUNCTION  (...) SET */
+	else if (Matches5("ALTER", "FUNCTION", MatchAny, MatchAny, "SET"))
+		COMPLETE_WITH_QUERY(Query_for_list_of_set_vars "UNION SELECT 'SCHEMA'");
+
 	/* ALTER SCHEMA  */
 	else if (Matches3("ALTER", "SCHEMA", MatchAny))
 		COMPLETE_WITH_LIST2("OWNER TO", "RENAME TO");
@@ -2705,7 +2717,9 @@ psql_completion(const char *text, int start, int end)
 
 /* SET, RESET, SHOW */
 	/* Complete with a variable name */
-	else if (TailMatches1("SET|RESET") && !TailMatches3("UPDATE", MatchAny, "SET"))
+	else if (TailMatches1("SET|RESET") &&
+			 !TailMatches3("UPDATE", MatchAny, "SET") &&
+			 !TailMatches2("ALTER", "FUNCTION"))
 		COMPLETE_WITH_QUERY(Query_for_list_of_set_vars);
 	else if (Matches1("SHOW"))
 		COMPLETE_WITH_QUERY(Query_for_list_of_show_vars);

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-02-01 Thread Fujii Masao
On Mon, Feb 1, 2016 at 9:23 PM, Michael Paquier
 wrote:
> On Mon, Feb 1, 2016 at 9:15 PM, Fujii Masao wrote:
>> If we do that, we also should change the tab-completion for SET command
>> so that "=" is suggested. But I'm afraid that which might decrease that
>> tab-completion.
>>
>> Imagine the case of "SET work_mem ". If "TO" and "=" are suggested,
>> we need to type either "T" or "=" and then  to input the setting value.
>> Otherwise, "SET work_mem " suggests only "TO" and we can input
>> the setting value by just typing  again.
>> This extra step is very small, but SET command is usually used very often,
>> so I'd like to avoid such extra step.
>
> OK, that's fine.

Pushed. Thanks!

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-02-01 Thread Michael Paquier
On Mon, Feb 1, 2016 at 9:15 PM, Fujii Masao wrote:
> If we do that, we also should change the tab-completion for SET command
> so that "=" is suggested. But I'm afraid that which might decrease that
> tab-completion.
>
> Imagine the case of "SET work_mem ". If "TO" and "=" are suggested,
> we need to type either "T" or "=" and then  to input the setting value.
> Otherwise, "SET work_mem " suggests only "TO" and we can input
> the setting value by just typing  again.
> This extra step is very small, but SET command is usually used very often,
> so I'd like to avoid such extra step.

OK, that's fine.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-02-01 Thread Fujii Masao
On Mon, Feb 1, 2016 at 3:14 PM, Michael Paquier
 wrote:
> On Mon, Feb 1, 2016 at 1:21 PM, Fujii Masao  wrote:
>> On Fri, Jan 29, 2016 at 1:02 PM, Michael Paquier
>>  wrote:
>>> On Fri, Jan 29, 2016 at 11:53 AM, Fujii Masao  wrote:
 I removed the above and added the following for that case.

 +/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
 +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.
>>
>> Thanks for the review!
>>
>>> "ALTER FUNCTION foo(bar)" suggests OWNER TO, RENAME TO and SET SCHEMA.
>>> I think that we had better suggesting SET instead of SET SCHEMA, and
>>> add SCHEMA in the list of things suggested by SET.
>>
>> Maybe, and it should suggest other keywords like RESET. That's it's better to
>> overhaul the tab-completion of ALTER FUNCTION. But that's not the task of
>> this patch. IMO it's better to fix that as a separate patch.
>
> Er, OK... I thought that both problems seem rather linked per the
> $subject but I can send an extra patch on this thread if necessary.
> Never mind.
>
>>> "ALTER DATABASE foodb SET foo_param" should suggest TO/= but that's
>>> not the case. After adding TO/= manually, a list of values is
>>> suggested though. Same problem with ALTER ROLE and ALTER FUNCTION.
>>
>> Fixed. Attached is the updated version of the patch.
>
> +   /* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
> +   else if (HeadMatches2("ALTER", "DATABASE|FUNCTION|ROLE|USER") &&
> +TailMatches2("SET", MatchAny))
> +   COMPLETE_WITH_LIST2("FROM CURRENT", "TO");
> Small thing: adding "=" to the list of things that can be completed?

If we do that, we also should change the tab-completion for SET command
so that "=" is suggested. But I'm afraid that which might decrease that
tab-completion.

Imagine the case of "SET work_mem ". If "TO" and "=" are suggested,
we need to type either "T" or "=" and then  to input the setting value.
Otherwise, "SET work_mem " suggests only "TO" and we can input
the setting value by just typing  again.
This extra step is very small, but SET command is usually used very often,
so I'd like to avoid such extra step.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-31 Thread Michael Paquier
On Mon, Feb 1, 2016 at 1:21 PM, Fujii Masao  wrote:
> On Fri, Jan 29, 2016 at 1:02 PM, Michael Paquier
>  wrote:
>> On Fri, Jan 29, 2016 at 11:53 AM, Fujii Masao  wrote:
>>> I removed the above and added the following for that case.
>>>
>>> +/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
>>> +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.
>
> Thanks for the review!
>
>> "ALTER FUNCTION foo(bar)" suggests OWNER TO, RENAME TO and SET SCHEMA.
>> I think that we had better suggesting SET instead of SET SCHEMA, and
>> add SCHEMA in the list of things suggested by SET.
>
> Maybe, and it should suggest other keywords like RESET. That's it's better to
> overhaul the tab-completion of ALTER FUNCTION. But that's not the task of
> this patch. IMO it's better to fix that as a separate patch.

Er, OK... I thought that both problems seem rather linked per the
$subject but I can send an extra patch on this thread if necessary.
Never mind.

>> "ALTER DATABASE foodb SET foo_param" should suggest TO/= but that's
>> not the case. After adding TO/= manually, a list of values is
>> suggested though. Same problem with ALTER ROLE and ALTER FUNCTION.
>
> Fixed. Attached is the updated version of the patch.

+   /* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
+   else if (HeadMatches2("ALTER", "DATABASE|FUNCTION|ROLE|USER") &&
+TailMatches2("SET", MatchAny))
+   COMPLETE_WITH_LIST2("FROM CURRENT", "TO");
Small thing: adding "=" to the list of things that can be completed?

Except that the rest looks fine to me.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-31 Thread Fujii Masao
On Fri, Jan 29, 2016 at 1:02 PM, Michael Paquier
 wrote:
> On Fri, Jan 29, 2016 at 11:53 AM, Fujii Masao  wrote:
>> I removed the above and added the following for that case.
>>
>> +/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
>> +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.

Thanks for the review!

> "ALTER FUNCTION foo(bar)" suggests OWNER TO, RENAME TO and SET SCHEMA.
> I think that we had better suggesting SET instead of SET SCHEMA, and
> add SCHEMA in the list of things suggested by SET.

Maybe, and it should suggest other keywords like RESET. That's it's better to
overhaul the tab-completion of ALTER FUNCTION. But that's not the task of
this patch. IMO it's better to fix that as a separate patch.

> "ALTER DATABASE foodb SET foo_param" should suggest TO/= but that's
> not the case. After adding TO/= manually, a list of values is
> suggested though. Same problem with ALTER ROLE and ALTER FUNCTION.

Fixed. Attached is the updated version of 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  */
! 	else if (Matches4("ALTER", "SYSTEM", "SET|RESET", MatchAny))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW  */
  	else if (Matches3("ALTER", "VIEW", MatchAny))
--- 1553,1559 
  	else if (Matches2("ALTER", "SYSTEM"))
  		COMPLETE_WITH_LIST2("SET", "RESET");
  	/* ALTER SYSTEM SET|RESET  */
! 	else if (Matches3("ALTER", "SYSTEM", "SET|RESET"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW  */
  	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  SET WITH provide OIDS */
+ 	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "WITH"))
+ 		COMPLETE_WITH_CONST("OIDS");
  	/* If we have ALTER TABLE  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  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  with "TO" */
  	else if (Matches2("SET", MatchAny))
  		COMPLETE_WITH_CONST("TO");
+ 	/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
+ 	else if (HeadMatches2("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


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-28 Thread Michael Paquier
On Fri, Jan 29, 2016 at 11:53 AM, Fujii Masao  wrote:
> I removed the above and added the following for that case.
>
> +/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
> +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.

"ALTER FUNCTION foo(bar)" suggests OWNER TO, RENAME TO and SET SCHEMA.
I think that we had better suggesting SET instead of SET SCHEMA, and
add SCHEMA in the list of things suggested by SET.

"ALTER DATABASE foodb SET foo_param" should suggest TO/= but that's
not the case. After adding TO/= manually, a list of values is
suggested though. Same problem with ALTER ROLE and ALTER FUNCTION.

> I also added the change that Sawada suggested, to the patch.

OK.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-28 Thread Fujii Masao
On Thu, Jan 28, 2016 at 10:50 PM, Masahiko Sawada  wrote:
> On Thu, Jan 28, 2016 at 10:15 PM, Michael Paquier
>  wrote:
>> On Thu, Jan 28, 2016 at 9:32 PM, Fujii Masao  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  lists nothing.
>>> 2. ALTER DATABASE xxx SET  lists nothing.
>>> 3. ALTER DATABASE xxx SET yyy  lists nothing.
>>> 4. ALTER DATABASE xxx SET datestyle TO  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 change breaks tab completion for ALTER TABLE ... SET
> [WITH/LOGGED/UNLOGGED].

Thanks for the review!
Fixed.

> It think it should be
>> +   else if (Matches2("SET", MatchAny) &&
>
> Related to it, I found tab completion for ALTER TABLE .. SET WITH,
> which doesn't working well.
> Patch is attached.

Please see the latest patch that I posted upthread.
I included your patch in that patch.

Regards,

-- 
Fujii Masao


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-28 Thread Fujii Masao
On Thu, Jan 28, 2016 at 10:15 PM, Michael Paquier
 wrote:
> On Thu, Jan 28, 2016 at 9:32 PM, Fujii Masao  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  lists nothing.
>> 2. ALTER DATABASE xxx SET  lists nothing.
>> 3. ALTER DATABASE xxx SET yyy  lists nothing.
>> 4. ALTER DATABASE xxx SET datestyle TO  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  */
+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  */
! 	else if (Matches4("ALTER", "SYSTEM", "SET|RESET", MatchAny))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW  */
  	else if (Matches3("ALTER", "VIEW", MatchAny))
--- 1553,1559 
  	else if (Matches2("ALTER", "SYSTEM"))
  		COMPLETE_WITH_LIST2("SET", "RESET");
  	/* ALTER SYSTEM SET|RESET  */
! 	else if (Matches3("ALTER", "SYSTEM", "SET|RESET"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW  */
  	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  SET WITH provide OIDS */
+ 	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "WITH"))
+ 		COMPLETE_WITH_CONST("OIDS");
  	/* If we have ALTER TABLE  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  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  with "TO" */
  	else if (Matches2("SET", MatchAny))
  		COMPLETE_WITH_CONST("TO");
+ 	/* Complete ALTER DATABASE|FUNCTION|ROLE|USER ... SET  */
+ 	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


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-28 Thread Masahiko Sawada
On Thu, Jan 28, 2016 at 10:15 PM, Michael Paquier
 wrote:
> On Thu, Jan 28, 2016 at 9:32 PM, Fujii Masao  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  lists nothing.
>> 2. ALTER DATABASE xxx SET  lists nothing.
>> 3. ALTER DATABASE xxx SET yyy  lists nothing.
>> 4. ALTER DATABASE xxx SET datestyle TO  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 change breaks tab completion for ALTER TABLE ... SET
[WITH/LOGGED/UNLOGGED].

It think it should be
> +   else if (Matches2("SET", MatchAny) &&

Related to it, I found tab completion for ALTER TABLE .. SET WITH,
which doesn't working well.
Patch is attached.

Regards,

--
Masahiko Sawada
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 008f3cb..033df74 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1758,6 +1758,8 @@ psql_completion(const char *text, int start, int end)
 	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "WITHOUT"))
 		COMPLETE_WITH_LIST2("CLUSTER", "OIDS");
 	/* ALTER TABLE  RESET */
+	else if (Matches5("ALTER", "TABLE", MatchAny, "SET", "WITH"))
+		COMPLETE_WITH_CONST("OIDS");
 	else if (Matches4("ALTER", "TABLE", MatchAny, "RESET"))
 		COMPLETE_WITH_CONST("(");
 	/* ALTER TABLE  SET|RESET ( */

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Several problems in tab-completions for SET/RESET

2016-01-28 Thread Michael Paquier
On Thu, Jan 28, 2016 at 9:32 PM, Fujii Masao  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  lists nothing.
> 2. ALTER DATABASE xxx SET  lists nothing.
> 3. ALTER DATABASE xxx SET yyy  lists nothing.
> 4. ALTER DATABASE xxx SET datestyle TO  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.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Several problems in tab-completions for SET/RESET

2016-01-28 Thread Fujii Masao
Hi,

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  lists nothing.
2. ALTER DATABASE xxx SET  lists nothing.
3. ALTER DATABASE xxx SET yyy  lists nothing.
4. ALTER DATABASE xxx SET datestyle TO  lists nothing.

Attached patch fixes those problems.

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  */
! 	else if (Matches4("ALTER", "SYSTEM", "SET|RESET", MatchAny))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW  */
  	else if (Matches3("ALTER", "VIEW", MatchAny))
--- 1553,1559 
  	else if (Matches2("ALTER", "SYSTEM"))
  		COMPLETE_WITH_LIST2("SET", "RESET");
  	/* ALTER SYSTEM SET|RESET  */
! 	else if (Matches3("ALTER", "SYSTEM", "SET|RESET"))
  		COMPLETE_WITH_QUERY(Query_for_list_of_alter_system_set_vars);
  	/* ALTER VIEW  */
  	else if (Matches3("ALTER", "VIEW", MatchAny))
***
*** 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);
--- 2702,2708 
  
  /* 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);
***
*** 2741,2750  psql_completion(const char *text, int start, int end)
  	else if (Matches2("RESET", "SESSION"))
  		COMPLETE_WITH_CONST("AUTHORIZATION");
  	/* Complete SET  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|="))
--- 2741,2754 
  	else if (Matches2("RESET", "SESSION"))
  		COMPLETE_WITH_CONST("AUTHORIZATION");
  	/* Complete SET  with "TO" */
! 	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");
  	/* 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