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

From
Subject RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
Date
Msg-id 7559e8ee8a0644c8aad79837092143c7@MP-MSGSS-MBX001.msg.nttdata.co.jp
Whole thread Raw
In response to Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion  (Masahiko Sawada <sawada.mshk@gmail.com>)
Responses Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
List pgsql-hackers
Thank you for your review!
I fixed some codes and attach a new patch.

>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.

>---
>-   /*
>-    * Complete FETCH <direction> with "FROM" or "IN". These are equivalent,
>-    * but we may as well tab-complete both: perhaps some users prefer one
>-    * variant or the other.
>-    */
>+   /* Complete FETCH <direction> with a list of cursors and "FROM" or
>+ "IN" */
>
>Why did you remove the second sentence in the above comment?

I had misunderstood the meaning and deleted it.
I deleted it as well as above, but would you prefer it to be there?

>---
>The patch improves tab completion for CLOSE, FETCH, and MOVE but is there
>any reason why you didn't do that for DECLARE? I think DECLARE also can be
>improved and it's a good timing for that.

I wanted to improve tab completion for DECLARE, but I couldn't find anything to improve.
Please let me know if there are any codes that can be improved.

Regards,
Shinya Kato

Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Reducing WaitEventSet syscall churn
Next
From: Craig Ringer
Date:
Subject: Re: [PATCH] LWLock self-deadlock detection