From: | <Shinya11(dot)Kato(at)nttdata(dot)com> |
---|---|
To: | <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | <pgsql-hackers(at)postgresql(dot)org> |
Subject: | RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion |
Date: | 2021-01-05 06:02:02 |
Message-ID: | 7559e8ee8a0644c8aad79837092143c7@MP-MSGSS-MBX001.msg.nttdata.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 | Content-Type | Size |
---|---|---|
fix_tab_complete_close_fetch_move_v2.patch | application/octet-stream | 2.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2021-01-05 06:04:17 | Re: [PATCH] LWLock self-deadlock detection |
Previous Message | Thomas Munro | 2021-01-05 05:10:09 | Re: Reducing WaitEventSet syscall churn |