[PATCH] Add tab-complete for backslash commands

Started by tanghy.fnst@fujitsu.comover 4 years ago11 messages

over 4 years ago

1 attachment(s)

Attached a patch to improve the tab completion for backslash commands.
I think it’s common for some people(I'm one of them) to use full-name commands than abbreviation.
So it's more convenient if we can add the full-name backslash commands in the tab-complete.c.

When modify tab-complete.c, I found \dS was added in the backslash_commands[], but I think maybe it should be removed just like other \x[S].
So I removed it.
Besides, I also added a little change in help.c.
- exchange the positon of \des and \det according to alphabetical order
- rename PATRN1/PATRN2 to ROLEPTRN/DBPTRN to make expression more comprehensible

Any comment is welcome.

Regards,
Tang

Attachments:

0001-add-tab-complete-for-backslash-commands.patchapplication/octet-stream; name=0001-add-tab-complete-for-backslash-commands.patchDownload

From 99c13bcedb358ad8140dbbff4cab22d97b4f0320 Mon Sep 17 00:00:00 2001
From: tanghy <tanghy.fnst@fujitsu.com>
Date: Thu, 15 Jul 2021 18:41:50 +0900
Subject: [PATCH] add tab-complete for backslash commands


diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d3fda67edd..3f8af98e6b 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -236,8 +236,8 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dD[S+] [PATTERN]      list domains\n"));
 	fprintf(output, _("  \\ddp    [PATTERN]      list default privileges\n"));
 	fprintf(output, _("  \\dE[S+] [PATTERN]      list foreign tables\n"));
-	fprintf(output, _("  \\det[+] [PATTERN]      list foreign tables\n"));
 	fprintf(output, _("  \\des[+] [PATTERN]      list foreign servers\n"));
+	fprintf(output, _("  \\det[+] [PATTERN]      list foreign tables\n"));
 	fprintf(output, _("  \\deu[+] [PATTERN]      list user mappings\n"));
 	fprintf(output, _("  \\dew[+] [PATTERN]      list foreign-data wrappers\n"));
 	fprintf(output, _("  \\df[anptw][S+] [FUNCPTRN [TYPEPTRN ...]]\n"
@@ -257,7 +257,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dO[S+] [PATTERN]      list collations\n"));
 	fprintf(output, _("  \\dp     [PATTERN]      list table, view, and sequence access privileges\n"));
 	fprintf(output, _("  \\dP[itn+] [PATTERN]    list [only index/table] partitioned relations [n=nested]\n"));
-	fprintf(output, _("  \\drds [PATRN1 [PATRN2]] list per-database role settings\n"));
+	fprintf(output, _("  \\drds [ROLEPTRN [DBPTRN]] list per-database role settings\n"));
 	fprintf(output, _("  \\dRp[+] [PATTERN]      list replication publications\n"));
 	fprintf(output, _("  \\dRs[+] [PATTERN]      list replication subscriptions\n"));
 	fprintf(output, _("  \\ds[S+] [PATTERN]      list sequences\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d6bf725971..91da49f7ed 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1510,24 +1510,24 @@ psql_completion(const char *text, int start, int end)
 		"\\des", "\\det", "\\deu", "\\dew", "\\dE", "\\df",
 		"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
 		"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\dP", "\\dPi", "\\dPt",
-		"\\drds", "\\dRs", "\\dRp", "\\ds", "\\dS",
+		"\\drds", "\\dRs", "\\dRp", "\\ds",
 		"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dX", "\\dy",
-		"\\e", "\\echo", "\\ef", "\\elif", "\\else", "\\encoding",
+		"\\e", "\\echo", "\\edit", "\\ef", "\\elif", "\\else", "\\encoding",
 		"\\endif", "\\errverbose", "\\ev",
 		"\\f",
 		"\\g", "\\gdesc", "\\gexec", "\\gset", "\\gx",
-		"\\h", "\\help", "\\H",
-		"\\i", "\\if", "\\ir",
-		"\\l", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
-		"\\o",
-		"\\p", "\\password", "\\prompt", "\\pset",
-		"\\q", "\\qecho",
-		"\\r",
+		"\\h", "\\help", "\\html", "\\H",
+		"\\i", "\\if", "\\ir", "\\include", "\\include_relative",
+		"\\l", "\\list", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
+		"\\o", "\\out",
+		"\\p", "\\password", "\\print", "\\prompt", "\\pset",
+		"\\q", "\\qecho", "\\quit",
+		"\\r", "\\rset",
 		"\\s", "\\set", "\\setenv", "\\sf", "\\sv",
 		"\\t", "\\T", "\\timing",
 		"\\unset",
 		"\\x",
-		"\\w", "\\warn", "\\watch",
+		"\\w", "\\warn", "\\watch", "\\write",
 		"\\z",
 		"\\!", "\\?",
 		NULL
-- 
2.31.1.windows.1

tanghy.fnst@fujitsu.com

over 4 years ago

In reply to: tanghy.fnst@fujitsu.com (#1)

RE: [PATCH] Add tab-complete for backslash commands

On Thursday, July 15, 2021 6:46 PM, tanghy.fnst@fujitsu.com <tanghy.fnst@fujitsu.com> wrote:

Attached a patch to improve the tab completion for backslash commands.
I think it's common for some people(I'm one of them) to use full-name commands than abbreviation.
So it's more convenient if we can add the full-name backslash commands in the tab-complete.c.

Add above patch in the commit fest as follows:

https://commitfest.postgresql.org/34/3268/

Regards,
Tang

Noname

ilmari@ilmari.org

over 4 years ago

In reply to: tanghy.fnst@fujitsu.com (#1)

1 attachment(s)

Re: [PATCH] Add tab-complete for backslash commands

Hi Tang,

"tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com> writes:

Hi

Attached a patch to improve the tab completion for backslash commands.
I think it’s common for some people(I'm one of them) to use full-name
commands than abbreviation. So it's more convenient if we can add the
full-name backslash commands in the tab-complete.c.

Even though I usually use the short versions, I agree that having the
full names in tab the completion as well is a good idea.

When modify tab-complete.c, I found \dS was added in the
backslash_commands[], but I think maybe it should be removed just like
other \x[S].
So I removed it.
Besides, I also added a little change in help.c.
- exchange the positon of \des and \det according to alphabetical order
- rename PATRN1/PATRN2 to ROLEPTRN/DBPTRN to make expression more comprehensible

These are also good changes.

Any comment is welcome.

Regards,
Tang

+ "\\r", "\\rset",

There's a typo here, that should be "\\reset". Also, I noticed that for
\connect, the situation is the opposite: it has the full form but not
the short form (\c).

I've addressed both in the attached v2 patch.

- ilmari

Attachments:

v2-0001-Add-tab-completion-for-more-backslash-commands.patchtext/x-diffDownload

From b8809f7ce81d6252712e8f115e4fb2edd551b7ea Mon Sep 17 00:00:00 2001
From: tanghy <tanghy.fnst@fujitsu.com>
Date: Sun, 8 Aug 2021 00:05:27 +0100
Subject: [PATCH v2] Add tab completion for more backslash commands

Also improve the --help output for some backslash commands
---
 src/bin/psql/help.c         |  4 ++--
 src/bin/psql/tab-complete.c | 22 +++++++++++-----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index d3fda67edd..3f8af98e6b 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -236,8 +236,8 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dD[S+] [PATTERN]      list domains\n"));
 	fprintf(output, _("  \\ddp    [PATTERN]      list default privileges\n"));
 	fprintf(output, _("  \\dE[S+] [PATTERN]      list foreign tables\n"));
-	fprintf(output, _("  \\det[+] [PATTERN]      list foreign tables\n"));
 	fprintf(output, _("  \\des[+] [PATTERN]      list foreign servers\n"));
+	fprintf(output, _("  \\det[+] [PATTERN]      list foreign tables\n"));
 	fprintf(output, _("  \\deu[+] [PATTERN]      list user mappings\n"));
 	fprintf(output, _("  \\dew[+] [PATTERN]      list foreign-data wrappers\n"));
 	fprintf(output, _("  \\df[anptw][S+] [FUNCPTRN [TYPEPTRN ...]]\n"
@@ -257,7 +257,7 @@ slashUsage(unsigned short int pager)
 	fprintf(output, _("  \\dO[S+] [PATTERN]      list collations\n"));
 	fprintf(output, _("  \\dp     [PATTERN]      list table, view, and sequence access privileges\n"));
 	fprintf(output, _("  \\dP[itn+] [PATTERN]    list [only index/table] partitioned relations [n=nested]\n"));
-	fprintf(output, _("  \\drds [PATRN1 [PATRN2]] list per-database role settings\n"));
+	fprintf(output, _("  \\drds [ROLEPTRN [DBPTRN]] list per-database role settings\n"));
 	fprintf(output, _("  \\dRp[+] [PATTERN]      list replication publications\n"));
 	fprintf(output, _("  \\dRs[+] [PATTERN]      list replication subscriptions\n"));
 	fprintf(output, _("  \\ds[S+] [PATTERN]      list sequences\n"));
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 064892bade..59caafda89 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1503,31 +1503,31 @@ psql_completion(const char *text, int start, int end)
 	/* psql's backslash commands. */
 	static const char *const backslash_commands[] = {
 		"\\a",
-		"\\connect", "\\conninfo", "\\C", "\\cd", "\\copy",
+		"\\c", "\\connect", "\\conninfo", "\\C", "\\cd", "\\copy",
 		"\\copyright", "\\crosstabview",
 		"\\d", "\\da", "\\dA", "\\dAc", "\\dAf", "\\dAo", "\\dAp",
 		"\\db", "\\dc", "\\dC", "\\dd", "\\ddp", "\\dD",
 		"\\des", "\\det", "\\deu", "\\dew", "\\dE", "\\df",
 		"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
 		"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\dP", "\\dPi", "\\dPt",
-		"\\drds", "\\dRs", "\\dRp", "\\ds", "\\dS",
+		"\\drds", "\\dRs", "\\dRp", "\\ds",
 		"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dX", "\\dy",
-		"\\e", "\\echo", "\\ef", "\\elif", "\\else", "\\encoding",
+		"\\e", "\\echo", "\\edit", "\\ef", "\\elif", "\\else", "\\encoding",
 		"\\endif", "\\errverbose", "\\ev",
 		"\\f",
 		"\\g", "\\gdesc", "\\gexec", "\\gset", "\\gx",
-		"\\h", "\\help", "\\H",
-		"\\i", "\\if", "\\ir",
-		"\\l", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
-		"\\o",
-		"\\p", "\\password", "\\prompt", "\\pset",
-		"\\q", "\\qecho",
-		"\\r",
+		"\\h", "\\help", "\\html", "\\H",
+		"\\i", "\\if", "\\ir", "\\include", "\\include_relative",
+		"\\l", "\\list", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
+		"\\o", "\\out",
+		"\\p", "\\password", "\\print", "\\prompt", "\\pset",
+		"\\q", "\\qecho", "\\quit",
+		"\\r", "\\reset",
 		"\\s", "\\set", "\\setenv", "\\sf", "\\sv",
 		"\\t", "\\T", "\\timing",
 		"\\unset",
 		"\\x",
-		"\\w", "\\warn", "\\watch",
+		"\\w", "\\warn", "\\watch", "\\write",
 		"\\z",
 		"\\!", "\\?",
 		NULL
-- 
2.30.2

tanghy.fnst@fujitsu.com

over 4 years ago

In reply to: Noname (#3)

RE: [PATCH] Add tab-complete for backslash commands

On Sunday, August 8, 2021 8:13 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

+ "\\r", "\\rset",

There's a typo here, that should be "\\reset". Also, I noticed that for
\connect, the situation is the opposite: it has the full form but not
the short form (\c).

I've addressed both in the attached v2 patch.

Thanks for you comments and fix. Your modified patch LGTM.

Regards,
Tang

Noname

ilmari@ilmari.org

over 4 years ago

In reply to: tanghy.fnst@fujitsu.com (#4)

Re: [PATCH] Add tab-complete for backslash commands

"tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com> writes:

On Sunday, August 8, 2021 8:13 AM, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

+ "\\r", "\\rset",

There's a typo here, that should be "\\reset". Also, I noticed that for
\connect, the situation is the opposite: it has the full form but not
the short form (\c).

I've addressed both in the attached v2 patch.

Thanks for you comments and fix. Your modified patch LGTM.

I've updated the commitfest entry to Ready for Committer.

Regards,
Tang

- ilmari

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: Noname (#5)

Re: [PATCH] Add tab-complete for backslash commands

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:

I've updated the commitfest entry to Ready for Committer.

I was about to push this but started to have second thoughts about it.
I'm not convinced that offering multiple variant spellings of the same
command is really such a great thing: I'm afraid that it will be more
confusing than helpful. I particularly question why we'd offer both
single- and multiple-character versions, as the single-character
version seems entirely useless from a completion standpoint.

For example, up to now "\o<TAB>" got you "\o ", which isn't amazingly
useful but maybe it serves to confirm that you typed a valid command.
This patch now forces you to choose between alternative spellings
of the exact same command, which is a waste of effort plus it will
make you stop to wonder whether they really are the same command.
It would be much better to either keep the old behavior, or just
immediately complete to "\out " and stay out of the user's way.

So I'd be inclined to take out the single-character versions of any
commands that we offer a longer spelling of. I'm not dead set on that,
but I think the possibility ought to be discussed.

regards, tom lane

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: Tom Lane (#6)

Re: [PATCH] Add tab-complete for backslash commands

... BTW, I went ahead and pushed the changes in help.c,
since that part seemed uncontroversial.

regards, tom lane

tanghy.fnst@fujitsu.com

over 4 years ago

In reply to: Tom Lane (#6)

RE: [PATCH] Add tab-complete for backslash commands

On Sunday, September 5, 2021 1:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I particularly question why we'd offer both
single- and multiple-character versions, as the single-character
version seems entirely useless from a completion standpoint.

I generally agreed with your opinion. But I'm not sure if there's someone
who'd like to see the list of backslash commands and choose one to use.
I mean, someone may type '\[tab][tab]' to check all supported backslash commands.
postgres=# \
Display all 105 possibilities? (y or n)
\! \dc \dL \dx \h \r
...

In the above scenario, both single- and multiple-character versions could be helpful, thought?

Regards,
Tang

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: tanghy.fnst@fujitsu.com (#8)

Re: [PATCH] Add tab-complete for backslash commands

"tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com> writes:

On Sunday, September 5, 2021 1:42 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I particularly question why we'd offer both
single- and multiple-character versions, as the single-character
version seems entirely useless from a completion standpoint.

I generally agreed with your opinion. But I'm not sure if there's someone
who'd like to see the list of backslash commands and choose one to use.
I mean, someone may type '\[tab][tab]' to check all supported backslash commands.

Sure, but he'd still get all the commands, just not all the possible
spellings of each one. And a person who's not sure what's available
is unlikely to be helped by an entry for "\c", because it's entirely
not clear which command that's an abbreviation for.

In any case, my main point is that the primary usage of tab-completion
is as a typing aid, not documentation. I do not think we should make
the behavior less useful for typing in order to be exhaustive on the
documentation side.

regards, tom lane

#10

tanghy.fnst@fujitsu.com

over 4 years ago

In reply to: Tom Lane (#9)

1 attachment(s)

RE: [PATCH] Add tab-complete for backslash commands

On Wednesday, September 8, 2021 5:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Sure, but he'd still get all the commands, just not all the possible
spellings of each one. And a person who's not sure what's available
is unlikely to be helped by an entry for "\c", because it's entirely
not clear which command that's an abbreviation for.

In any case, my main point is that the primary usage of tab-completion
is as a typing aid, not documentation. I do not think we should make
the behavior less useful for typing in order to be exhaustive on the
documentation side.

You are right. I think I've got your point.
Here is the updated patch in which I added the multiple-character versions for backslash commands
and remove their corresponding single-character version.
Of course, for backslash commands with only single-character version, no change added.

BTW, I've done the existing tap-tests for tab-completion with this patch, all tests passed.

Regards,
Tang

Attachments:

v3-0001-Replace-the-single-character-with-multiple-charac.patchapplication/octet-stream; name=v3-0001-Replace-the-single-character-with-multiple-charac.patchDownload

From c3301f89f33facfa6f2b99a2a2d823c7a80382a2 Mon Sep 17 00:00:00 2001
From: tanghy <tanghy.fnst@fujitsu.com>
Date: Wed, 8 Sep 2021 18:38:25 +0900
Subject: [PATCH v3] Replace the single-character with multiple-character
 backslash commands in tab completion.

---
 src/bin/psql/tab-complete.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 75b867685a..9c01594af6 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1514,24 +1514,24 @@ psql_completion(const char *text, int start, int end)
 		"\\des", "\\det", "\\deu", "\\dew", "\\dE", "\\df",
 		"\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL",
 		"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\dP", "\\dPi", "\\dPt",
-		"\\drds", "\\dRs", "\\dRp", "\\ds", "\\dS",
+		"\\drds", "\\dRs", "\\dRp", "\\ds",
 		"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dX", "\\dy",
-		"\\e", "\\echo", "\\ef", "\\elif", "\\else", "\\encoding",
+		"\\echo", "\\edit", "\\ef", "\\elif", "\\else", "\\encoding",
 		"\\endif", "\\errverbose", "\\ev",
 		"\\f",
 		"\\g", "\\gdesc", "\\gexec", "\\gset", "\\gx",
-		"\\h", "\\help", "\\H",
-		"\\i", "\\if", "\\ir",
-		"\\l", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
-		"\\o",
-		"\\p", "\\password", "\\prompt", "\\pset",
-		"\\q", "\\qecho",
-		"\\r",
+		"\\help", "\\html",
+		"\\if", "\\include", "\\include_relative",
+		"\\list", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
+		"\\out",
+		"\\password", "\\print", "\\prompt", "\\pset",
+		"\\qecho", "\\quit",
+		"\\reset",
 		"\\s", "\\set", "\\setenv", "\\sf", "\\sv",
 		"\\t", "\\T", "\\timing",
 		"\\unset",
 		"\\x",
-		"\\w", "\\warn", "\\watch",
+		"\\warn", "\\watch", "\\write",
 		"\\z",
 		"\\!", "\\?",
 		NULL
-- 
2.31.1.windows.1

#11

Tom Lane

tgl@sss.pgh.pa.us

over 4 years ago

In reply to: tanghy.fnst@fujitsu.com (#10)

Re: [PATCH] Add tab-complete for backslash commands

"tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com> writes:

Here is the updated patch in which I added the multiple-character versions for backslash commands
and remove their corresponding single-character version.
Of course, for backslash commands with only single-character version, no change added.

Pushed. I tweaked your list to the extent of adding back "\ir",
because since it's two letters not one, the argument that it's
entirely useless for tab completion doesn't quite apply. But
if we wanted to make a hard-and-fast policy of offering only
the long form when there are two forms, maybe we should remove
that one too.

regards, tom lane