From: Shinya11.Kato@nttdata.com
To: sawada.mshk@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
Date: Tue, 5 Jan 2021 06:02:02 +0000
Message-ID: <7559e8ee8a0644c8aad79837092143c7@MP-MSGSS-MBX001.msg.nttdata.co.jp> (raw)
In-Reply-To: <CAD21AoBRw9FY6oNj8ubY7XEwJ9Bprfehu88_ePbCfH93JLxWsw@mail.gmail.com>
References: <b0e4c5c53ef84c5395524f5056fc71f0@MP-MSGSS-MBX001.msg.nttdata.co.jp>
<CAD21AoBRw9FY6oNj8ubY7XEwJ9Bprfehu88_ePbCfH93JLxWsw@mail.gmail.com>
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
Attachments:fix_tab_complete_close_fetch_move_v2.patch (application/octet-stream, 2.8K) downloaddiff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.cindex 3a43c09bf6..96f3fe8b9f 100644--- a/src/bin/psql/tab-complete.c+++ b/src/bin/psql/tab-complete.c@@ -976,6 +976,9 @@ static const SchemaQuery Query_for_list_of_statistics = {
" and pg_catalog.pg_table_is_visible(c2.oid)"\
" and c2.relispartition = 'true'"
+#define Query_for_list_of_cursors \+" SELECT name FROM pg_cursors"\+
/*
* These object types were introduced later than our support cutoff of
* server version 7.4. We use the VersionedQuery infrastructure so that
@@ -2284,6 +2287,10 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL);
else if (Matches("CALL", MatchAny))
COMPLETE_WITH("(");
+/* CLOSE */+ else if (Matches("CLOSE"))+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors+ " UNION ALL SELECT 'ALL'");
/* CLUSTER */
else if (Matches("CLUSTER"))
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_clusterables, "UNION SELECT 'VERBOSE'");
@@ -3164,28 +3171,33 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
/* FETCH && MOVE */
-- /*- * Complete FETCH with one of ABSOLUTE, BACKWARD, FORWARD, RELATIVE, ALL,- * NEXT, PRIOR, FIRST, LAST- */
else if (Matches("FETCH|MOVE"))
- COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",- "ALL", "NEXT", "PRIOR", "FIRST", "LAST");-- /* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */+ 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'");
else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
- COMPLETE_WITH("ALL", "FROM", "IN");-- /*- * 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_WITH_QUERY(Query_for_list_of_cursors+ " UNION SELECT 'ALL'"+ " UNION SELECT 'FROM'"+ " UNION SELECT 'IN'");
else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAnyExcept("FROM|IN")) ||
Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
- COMPLETE_WITH("FROM", "IN");+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors+ " UNION SELECT 'FROM'"+ " UNION SELECT 'IN'");+ else if (HeadMatches("FETCH|MOVE") &&+ TailMatches("FROM|IN"))+ COMPLETE_WITH_QUERY(Query_for_list_of_cursors);
/* FOREIGN DATA WRAPPER */
/* applies in ALTER/DROP FDW and in CREATE SERVER */
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: Shinya11.Kato@nttdata.com, sawada.mshk@gmail.com
Subject: RE: [PATCH] Feature improvement for CLOSE, FETCH, MOVE tab completion
In-Reply-To: <7559e8ee8a0644c8aad79837092143c7@MP-MSGSS-MBX001.msg.nttdata.co.jp>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox