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

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Several problems in tab-completions for SET/RESET
Date: 2016-02-01 12:15:20
Message-ID: CAHGQGwFo--wp9NQMxd53uaaUkEkvzJ9RBRa81yAjH+HfSMb1wg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Feb 1, 2016 at 3:14 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Mon, Feb 1, 2016 at 1:21 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Fri, Jan 29, 2016 at 1:02 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> On Fri, Jan 29, 2016 at 11:53 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>> 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.
>>
>> 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 <name> */
> + 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 <tab>". If "TO" and "=" are suggested,
we need to type either "T" or "=" and then <tab> to input the setting value.
Otherwise, "SET work_mem <tab>" suggests only "TO" and we can input
the setting value by just typing <tab> 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2016-02-01 12:18:32 Re: Template for commit messages
Previous Message Vitaly Burovoy 2016-02-01 12:07:51 Re: Patch: make behavior of all versions of the "isinf" function be similar