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

[PATCH] Add tab-complete for backslash commands

From
"tanghy.fnst@fujitsu.com"
Date:
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.

When modify tab-complete.c, I found \dS was added in the backslash_commands[], but I think maybe it should be removed
justlike 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

Attachment

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

From
"tanghy.fnst@fujitsu.com"
Date:
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




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

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
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

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


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

From
"tanghy.fnst@fujitsu.com"
Date:
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

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

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
"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



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

From
Tom Lane
Date:
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



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

From
Tom Lane
Date:
... BTW, I went ahead and pushed the changes in help.c,
since that part seemed uncontroversial.

            regards, tom lane



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

From
"tanghy.fnst@fujitsu.com"
Date:
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

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

From
Tom Lane
Date:
"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



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

From
"tanghy.fnst@fujitsu.com"
Date:
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

Attachment

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

From
Tom Lane
Date:
"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