Feature improvement for FETCH tab completion

Lists: pgsql-hackers
From: btnakamichin <btnakamichin(at)oss(dot)nttdata(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Feature improvement for FETCH tab completion
Date: 2020-09-25 05:24:30
Message-ID: d05a46b599634ca0d94144387507f4b4@oss.nttdata.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Hello!

I’d like to improve the FETCH tab completion.

The FETCH tab completion . Therefore, this patch fixes the problem.

Previous function completed one of FORWARD, BACKWARD, RELATIVE,
ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE,
ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and
FROM clauses.

Regards,

Naoki Nakamichi

Attachment Content-Type Size
fix_tab_complete_fetch.patch text/x-diff 1.5 KB

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: btnakamichin <btnakamichin(at)oss(dot)nttdata(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Feature improvement for FETCH tab completion
Date: 2020-09-25 06:38:46
Message-ID: bea7d871-affc-d190-f72f-5369aa663c6d@oss.nttdata.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 2020/09/25 14:24, btnakamichin wrote:
> Hello!
>
> I’d like to improve the FETCH tab completion.
>
> The FETCH tab completion . Therefore, this patch fixes the problem.
>
> Previous function completed one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and FROM clauses.

Thanks for the patch! Here are review comments.

+ /* Complete FETCH BACKWARD or FORWARD with one of ALL */
+ else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+ COMPLETE_WITH("ALL");

Not only "ALL" but also "FROM" and "IN" should be displayed here
because they also can follow "BACKWARD" and "FORWARD"?

else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+ else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE", MatchAny))
+ COMPLETE_WITH("FROM", "IN");

This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
and "IN". To avoid this confusing tab-completion, we should use something like
MatchAnyExcept("FROM|IN") here, instead?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


From: btnakamichin <btnakamichin(at)oss(dot)nttdata(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Feature improvement for FETCH tab completion
Date: 2020-09-25 08:21:29
Message-ID: e41e668e4e68dfd9e4a74ce98e54eb2c@oss.nttdata.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

2020-09-25 15:38 に Fujii Masao さんは書きました:
> On 2020/09/25 14:24, btnakamichin wrote:
>> Hello!
>>
>> I’d like to improve the FETCH tab completion.
>>
>> The FETCH tab completion . Therefore, this patch fixes the problem.
>>
>> Previous function completed one of FORWARD, BACKWARD, RELATIVE,
>> ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE,
>> ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN
>> and FROM clauses.
>
> Thanks for the patch! Here are review comments.
>
> + /* Complete FETCH BACKWARD or FORWARD with one of ALL */
> + else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
> + COMPLETE_WITH("ALL");
>
> Not only "ALL" but also "FROM" and "IN" should be displayed here
> because they also can follow "BACKWARD" and "FORWARD"?
>
> else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
> + else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
> MatchAny))
> + COMPLETE_WITH("FROM", "IN");
>
> This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
> and "IN". To avoid this confusing tab-completion, we should use
> something like
> MatchAnyExcept("FROM|IN") here, instead?
>
> Regards,

I’m Sorry, I forgot to include pgsql_hackers in the cc, so I resend it

Thank you, I appreciate your comment.

I have attached patch with newline.

Regards,

NaokiNakamichi

Attachment Content-Type Size
fix_tab_complete_fetch_v2.patch text/x-diff 2.0 KB

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: btnakamichin <btnakamichin(at)oss(dot)nttdata(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Feature improvement for FETCH tab completion
Date: 2020-09-25 12:46:20
Message-ID: 3a4f470b-f840-9918-8e99-2a43fe37510e@oss.nttdata.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 2020/09/25 17:21, btnakamichin wrote:
> 2020-09-25 15:38 に Fujii Masao さんは書きました:
>> On 2020/09/25 14:24, btnakamichin wrote:
>>> Hello!
>>>
>>> I’d like to improve the FETCH tab completion.
>>>
>>> The FETCH tab completion . Therefore, this patch fixes the problem.
>>>
>>> Previous function completed one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and FROM clauses.
>>
>> Thanks for the patch! Here are review comments.
>>
>> +    /* Complete FETCH BACKWARD or FORWARD with one of ALL */
>> +    else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
>> +        COMPLETE_WITH("ALL");
>>
>> Not only "ALL" but also "FROM" and "IN" should be displayed here
>> because they also can follow "BACKWARD" and "FORWARD"?
>>
>>     else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
>> +    else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
>> MatchAny))
>> +        COMPLETE_WITH("FROM", "IN");
>>
>> This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
>> and "IN". To avoid this confusing tab-completion, we should use something like
>> MatchAnyExcept("FROM|IN") here, instead?
>>
>> Regards,
>
> I’m Sorry, I forgot to include pgsql_hackers in the cc, so I resend it
>
> Thank you, I appreciate your comment.
>
> I have attached patch with newline.

Thanks for updating the patch!

The patch should include only the change of tab-complete for FETCH,
but accidentally it also includes DEALLOCATE that's proposed by you
in another thread. So I exclude DEALLOCATE part from the patch.
Attached is the updated version of the patch.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachment Content-Type Size
fix_tab_complete_fetch_v3.patch text/plain 1.6 KB

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: btnakamichin <btnakamichin(at)oss(dot)nttdata(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Feature improvement for FETCH tab completion
Date: 2020-10-15 07:53:03
Message-ID: 52bf8779-20e8-5113-4b82-111569ee33c8@oss.nttdata.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On 2020/09/25 21:46, Fujii Masao wrote:
>
>
> On 2020/09/25 17:21, btnakamichin wrote:
>> 2020-09-25 15:38 に Fujii Masao さんは書きました:
>>> On 2020/09/25 14:24, btnakamichin wrote:
>>>> Hello!
>>>>
>>>> I’d like to improve the FETCH tab completion.
>>>>
>>>> The FETCH tab completion . Therefore, this patch fixes the problem.
>>>>
>>>> Previous function completed one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and FROM clauses.
>>>
>>> Thanks for the patch! Here are review comments.
>>>
>>> +    /* Complete FETCH BACKWARD or FORWARD with one of ALL */
>>> +    else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
>>> +        COMPLETE_WITH("ALL");
>>>
>>> Not only "ALL" but also "FROM" and "IN" should be displayed here
>>> because they also can follow "BACKWARD" and "FORWARD"?
>>>
>>>     else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
>>> +    else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
>>> MatchAny))
>>> +        COMPLETE_WITH("FROM", "IN");
>>>
>>> This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
>>> and "IN". To avoid this confusing tab-completion, we should use something like
>>> MatchAnyExcept("FROM|IN") here, instead?
>>>
>>> Regards,
>>
>> I’m Sorry, I forgot to include pgsql_hackers in the cc, so I resend it
>>
>> Thank you, I appreciate your comment.
>>
>> I have attached patch with newline.
>
> Thanks for updating the patch!
>
> The patch should include only the change of tab-complete for FETCH,
> but accidentally it also includes DEALLOCATE that's proposed by you
> in another thread. So I exclude DEALLOCATE part from the patch.
> Attached is the updated version of the patch.

Pushed. Thanks!

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION