On Wed, Dec 9, 2020 at 12:59 PM <shinya11.k...@nttdata.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/