Re: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion

On Wed, Dec 9, 2020 at 12:59 PM <Shinya11(dot)Kato(at)nttdata(dot)com> wrote:
>
> Hi!
>
>
>
> I created a patch for improving CLOSE, FETCH, MOVE tab completion.
>
> Specifically, I add CLOSE, FETCH, MOVE tab completion for completing a predefined cursors.
>

Thank you for the 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.

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

---
- /*
- * 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?

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

Regards,

--
Masahiko Sawada
EnterpriseDB: https://www.enterprisedb.com/