Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion - Mailing list pgsql-hackers

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