From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Shinya11.Kato@nttdata.com
To: sawada.mshk@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
Date: Tue, 5 Jan 2021 18:08:21 +0900
Message-ID: <0eab839e-5827-e517-4abf-00b3e581cd45@oss.nttdata.com> (raw)
In-Reply-To: <7559e8ee8a0644c8aad79837092143c7@MP-MSGSS-MBX001.msg.nttdata.co.jp>
References: <b0e4c5c53ef84c5395524f5056fc71f0@MP-MSGSS-MBX001.msg.nttdata.co.jp>
<CAD21AoBRw9FY6oNj8ubY7XEwJ9Bprfehu88_ePbCfH93JLxWsw@mail.gmail.com>
<7559e8ee8a0644c8aad79837092143c7@MP-MSGSS-MBX001.msg.nttdata.co.jp>
On 2021/01/05 15:02, Shinya11.Kato@nttdata.com wrote:
> Thank you for your review!
> I fixed some codes and attach a new patch.
Thanks for updating the patch!
+#define Query_for_list_of_cursors \
+" SELECT name FROM pg_cursors"\
This query should be the following?
" SELECT pg_catalog.quote_ident(name) "\
" FROM pg_catalog.pg_cursors "\
" WHERE substring(pg_catalog.quote_ident(name),1,%d)='%s'"
+/* CLOSE */
+ else if (Matches("CLOSE"))
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION ALL SELECT 'ALL'");
"UNION ALL" should be "UNION"?
+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors
+ " UNION SELECT 'ABSOLUTE'"
+ " UNION SELECT 'BACKWARD'"
+ " UNION SELECT 'FORWARD'"
+ " UNION SELECT 'RELATIVE'"
+ " UNION SELECT 'ALL'"
+ " UNION SELECT 'NEXT'"
+ " UNION SELECT 'PRIOR'"
+ " UNION SELECT 'FIRST'"
+ " UNION SELECT 'LAST'"
+ " UNION SELECT 'FROM'"
+ " UNION SELECT 'IN'");
This change makes psql unexpectedly output "FROM" and "IN" just after "FETCH".
>
>> When I applied the patch, I got the following whitespace warnings:
>>
>> $ git apply ~/patches/fix_tab_complete_close_fetch_move.patch
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:40:
>> indent with spaces.
>> COMPLETE_WITH_QUERY(Query_for_list_of_cursors
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:41:
>> indent with spaces.
>> " UNION SELECT 'ABSOLUTE'"
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:42:
>> indent with spaces.
>> " UNION SELECT 'BACKWARD'"
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:43:
>> indent with spaces.
>> " UNION SELECT 'FORWARD'"
>> /home/masahiko/patches/fix_tab_complete_close_fetch_move.patch:44:
>> indent with spaces.
>> " UNION SELECT 'RELATIVE'"
>> warning: squelched 19 whitespace errors
>> warning: 24 lines add whitespace errors.
>>
>> I recommend you checking whitespaces or running pgindent.
>
> Thank you for your advice and I have corrected it.
>
>> Here are some comments:
>>
>> /*
>> - * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
>> RELATIVE, ALL,
>> - * NEXT, PRIOR, FIRST, LAST
>> + * Complete FETCH with a list of cursors and one of ABSOLUTE,
>> BACKWARD, FORWARD, RELATIVE, ALL,
>> + * NEXT, PRIOR, FIRST, LAST, FROM, IN
>> */
>>
>> Maybe I think the commend should say:
>>
>> + * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD,
>> RELATIVE, ALL,
>> + * NEXT, PRIOR, FIRST, LAST, FROM, IN, and a list of cursors
>>
>> Other updates of the comment seem to have the same issue.
>>
>> Or I think we can omit the details from the comment since it seems redundant
>> information. We can read it from the arguments of the following
>> COMPLETE_WITH_QUERY().
>
> It certainly seems redundant, so I deleted them.
I think that it's better to update and keep those comments rather than removing them.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, Shinya11.Kato@nttdata.com, sawada.mshk@gmail.com
Subject: Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
In-Reply-To: <0eab839e-5827-e517-4abf-00b3e581cd45@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox