Обсуждение: [PATCH] Add tab-complete for backslash commands
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
Вложения
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
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
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
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
От
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Дата:
"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
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
... BTW, I went ahead and pushed the changes in help.c, since that part seemed uncontroversial. regards, tom lane
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
"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
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
Вложения
"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