Feature improvement for FETCH tab completion

Started by btnakamichinover 5 years ago5 messages

btnakamichin

btnakamichin@oss.nttdata.com

over 5 years ago

1 attachment(s)

Hello!

I’d like to improve the FETCH tab completion.

The FETCH tab completion . Therefore, this patch fixes the problem.

Previous function completed one of FORWARD, BACKWARD, RELATIVE,
ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE,
ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and
FROM clauses.

Regards,

Naoki Nakamichi

Attachments:

fix_tab_complete_fetch.patchtext/x-diff; name=fix_tab_complete_fetch.patchDownload

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d877cc86f0..eb103b784e 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3076,19 +3076,22 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
 
 /* FETCH && MOVE */
-	/* Complete FETCH with one of FORWARD, BACKWARD, RELATIVE */
+	/* Complete FETCH with one of FORWARD, BACKWARD, RELATIVE, ALL, NEXT, PRIOR, FIRST, LAST */
 	else if (Matches("FETCH|MOVE"))
-		COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE");
-	/* Complete FETCH <sth> with one of ALL, NEXT, PRIOR */
-	else if (Matches("FETCH|MOVE", MatchAny))
-		COMPLETE_WITH("ALL", "NEXT", "PRIOR");
+		COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE", "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
+	/* Complete FETCH BACKWARD or FORWARD with one of ALL */
+	else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+		COMPLETE_WITH("ALL");
 
 	/*
-	 * Complete FETCH <sth1> <sth2> with "FROM" or "IN". These are equivalent,
+	 * 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.
 	 */
-	else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+	else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE", MatchAny))
+		COMPLETE_WITH("FROM", "IN");
+
+	else if (Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
 		COMPLETE_WITH("FROM", "IN");
 
 /* FOREIGN DATA WRAPPER */

Fujii Masao

masao.fujii@oss.nttdata.com

over 5 years ago

In reply to: btnakamichin (#1)

Re: Feature improvement for FETCH tab completion

On 2020/09/25 14:24, btnakamichin wrote:

Hello!

I’d like to improve the FETCH tab completion.

The FETCH tab completion . Therefore, this patch fixes the problem.

Previous function completed one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and FROM clauses.

Thanks for the patch! Here are review comments.

+	/* Complete FETCH BACKWARD or FORWARD with one of ALL */
+	else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+		COMPLETE_WITH("ALL");

Not only "ALL" but also "FROM" and "IN" should be displayed here
because they also can follow "BACKWARD" and "FORWARD"?

	else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+	else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE", MatchAny))
+		COMPLETE_WITH("FROM", "IN");

This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
and "IN". To avoid this confusing tab-completion, we should use something like
MatchAnyExcept("FROM|IN") here, instead?

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

btnakamichin

btnakamichin@oss.nttdata.com

over 5 years ago

In reply to: Fujii Masao (#2)

1 attachment(s)

Re: Feature improvement for FETCH tab completion

2020-09-25 15:38 に Fujii Masao さんは書きました:

On 2020/09/25 14:24, btnakamichin wrote:

Hello!

I’d like to improve the FETCH tab completion.

The FETCH tab completion . Therefore, this patch fixes the problem.

Previous function completed one of FORWARD, BACKWARD, RELATIVE,
ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE,
ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN
and FROM clauses.

Thanks for the patch! Here are review comments.
+	/* Complete FETCH BACKWARD or FORWARD with one of ALL */
+	else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+		COMPLETE_WITH("ALL");
Not only "ALL" but also "FROM" and "IN" should be displayed here
because they also can follow "BACKWARD" and "FORWARD"?
else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+	else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAny))
+		COMPLETE_WITH("FROM", "IN");
This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
and "IN". To avoid this confusing tab-completion, we should use
something like
MatchAnyExcept("FROM|IN") here, instead?

Regards,

I’m Sorry, I forgot to include pgsql_hackers in the cc, so I resend it

Thank you, I appreciate your comment.

I have attached patch　with newline.

Regards,

NaokiNakamichi

Attachments:

fix_tab_complete_fetch_v2.patchtext/x-diff; name=fix_tab_complete_fetch_v2.patchDownload

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d877cc86f0..dafbae0366 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2911,7 +2911,8 @@ psql_completion(const char *text, int start, int end)
 
 /* DEALLOCATE */
 	else if (Matches("DEALLOCATE"))
-		COMPLETE_WITH_QUERY(Query_for_list_of_prepared_statements " UNION SELECT 'ALL'");
+		COMPLETE_WITH_QUERY(Query_for_list_of_prepared_statements 
+		                    " UNION SELECT 'ALL'");
 
 /* DECLARE */
 	else if (Matches("DECLARE", MatchAny))
@@ -3076,19 +3077,27 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
 
 /* FETCH && MOVE */
-	/* Complete FETCH with one of FORWARD, BACKWARD, RELATIVE */
+	/*
+	 * Complete FETCH with one of FORWARD, BACKWARD, RELATIVE, ALL, NEXT,
+	 * PRIOR, FIRST, LAST
+	 */
 	else if (Matches("FETCH|MOVE"))
-		COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE");
-	/* Complete FETCH <sth> with one of ALL, NEXT, PRIOR */
-	else if (Matches("FETCH|MOVE", MatchAny))
-		COMPLETE_WITH("ALL", "NEXT", "PRIOR");
+		COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE", "ALL", "NEXT",
+		              "PRIOR", "FIRST", "LAST");
+	/* Complete FETCH BACKWARD or FORWARD with one of ALL, IN, FROM */
+	else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+		COMPLETE_WITH("ALL", "FROM", "IN");
 
 	/*
-	 * Complete FETCH <sth1> <sth2> with "FROM" or "IN". These are equivalent,
+	 * 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.
 	 */
-	else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+	else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
+	         MatchAnyExcept("FROM|IN")))
+		COMPLETE_WITH("FROM", "IN");
+
+	else if (Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
 		COMPLETE_WITH("FROM", "IN");
 
 /* FOREIGN DATA WRAPPER */

Fujii Masao

masao.fujii@oss.nttdata.com

over 5 years ago

In reply to: btnakamichin (#3)

1 attachment(s)

Re: Feature improvement for FETCH tab completion

On 2020/09/25 17:21, btnakamichin wrote:

2020-09-25 15:38 に Fujii Masao さんは書きました:
On 2020/09/25 14:24, btnakamichin wrote:

Hello!

I’d like to improve the FETCH tab completion.

The FETCH tab completion . Therefore, this patch fixes the problem.

Previous function completed one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and FROM clauses.

Thanks for the patch! Here are review comments.
+    /* Complete FETCH BACKWARD or FORWARD with one of ALL */
+    else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+        COMPLETE_WITH("ALL");
Not only "ALL" but also "FROM" and "IN" should be displayed here
because they also can follow "BACKWARD" and "FORWARD"?
    else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+    else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAny))
+        COMPLETE_WITH("FROM", "IN");
This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
and "IN". To avoid this confusing tab-completion, we should use something like
MatchAnyExcept("FROM|IN") here, instead?

Regards,
I’m Sorry, I forgot to include pgsql_hackers in the cc, so I resend it

Thank you, I appreciate your comment.

I have attached patch　with newline.

Thanks for updating the patch!

The patch should include only the change of tab-complete for FETCH,
but accidentally it also includes DEALLOCATE that's proposed by you
in another thread. So I exclude DEALLOCATE part from the patch.
Attached is the updated version of the patch.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachments:

fix_tab_complete_fetch_v3.patchtext/plain; charset=UTF-8; name=fix_tab_complete_fetch_v3.patch; x-mac-creator=0; x-mac-type=0Download

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9c6f5ecb6a..3cf7698c0b 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3076,19 +3076,27 @@ psql_completion(const char *text, int start, int end)
 		COMPLETE_WITH("SELECT", "INSERT", "DELETE", "UPDATE", "DECLARE");
 
 /* FETCH && MOVE */
-	/* Complete FETCH with one of FORWARD, BACKWARD, RELATIVE */
+
+	/*
+	 * 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");
-	/* Complete FETCH <sth> with one of ALL, NEXT, PRIOR */
-	else if (Matches("FETCH|MOVE", MatchAny))
-		COMPLETE_WITH("ALL", "NEXT", "PRIOR");
+		COMPLETE_WITH("ABSOLUTE", "BACKWARD", "FORWARD", "RELATIVE",
+					  "ALL", "NEXT", "PRIOR", "FIRST", "LAST");
+
+	/* Complete FETCH BACKWARD or FORWARD with one of ALL, FROM, IN */
+	else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+		COMPLETE_WITH("ALL", "FROM", "IN");
 
 	/*
-	 * Complete FETCH <sth1> <sth2> with "FROM" or "IN". These are equivalent,
+	 * 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.
 	 */
-	else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+	else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
+					 MatchAnyExcept("FROM|IN")) ||
+			 Matches("FETCH|MOVE", "ALL|NEXT|PRIOR|FIRST|LAST"))
 		COMPLETE_WITH("FROM", "IN");
 
 /* FOREIGN DATA WRAPPER */

Fujii Masao

masao.fujii@oss.nttdata.com

about 5 years ago

In reply to: Fujii Masao (#4)

Re: Feature improvement for FETCH tab completion

On 2020/09/25 21:46, Fujii Masao wrote:

On 2020/09/25 17:21, btnakamichin wrote:
2020-09-25 15:38 に Fujii Masao さんは書きました:
On 2020/09/25 14:24, btnakamichin wrote:

Hello!

I’d like to improve the FETCH tab completion.

The FETCH tab completion . Therefore, this patch fixes the problem.

Previous function completed one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, but now it completes one of FORWARD, BACKWARD, RELATIVE, ABSOLUTE, ALL, NEXT, PRIOR, FIRST, LAST and Corresponded to later IN and FROM clauses.

Thanks for the patch! Here are review comments.
+    /* Complete FETCH BACKWARD or FORWARD with one of ALL */
+    else if (Matches("FETCH|MOVE", "BACKWARD|FORWARD"))
+        COMPLETE_WITH("ALL");
Not only "ALL" but also "FROM" and "IN" should be displayed here
because they also can follow "BACKWARD" and "FORWARD"?
    else if (Matches("FETCH|MOVE", MatchAny, MatchAny))
+    else if (Matches("FETCH|MOVE", "ABSOLUTE|BACKWARD|FORWARD|RELATIVE",
MatchAny))
+        COMPLETE_WITH("FROM", "IN");
This change seems to cause "FETCH FORWARD FROM <tab>" to display "FROM"
and "IN". To avoid this confusing tab-completion, we should use something like
MatchAnyExcept("FROM|IN") here, instead?

Regards,
I’m Sorry, I forgot to include pgsql_hackers in the cc, so I resend it

Thank you, I appreciate your comment.

I have attached patch　with newline.
Thanks for updating the patch!

The patch should include only the change of tab-complete for FETCH,
but accidentally it also includes DEALLOCATE that's proposed by you
in another thread. So I exclude DEALLOCATE part from the patch.
Attached is the updated version of the patch.

Pushed. Thanks!

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION