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

On Thu, Jan 28, 2016 at 10:15 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Jan 28, 2016 at 9:32 PM, Fujii Masao <masao.fujii@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