psql: fix variable existence tab completion
Hello hackers,
psql has the :{?name} syntax for testing a psql variable existence.
But currently doing \echo :{?VERB<Tab> doesn't trigger tab completion.
This patch fixes it. I've also included a TAP test.
Best regards,
Steve Chavez
Attachments:
0001-psql-fix-variable-existence-tab-completion.patchtext/x-patch; charset=US-ASCII; name=0001-psql-fix-variable-existence-tab-completion.patchDownload
From adb1f997b67d8ef603017ab34e1b9061e4e229ea Mon Sep 17 00:00:00 2001
From: steve-chavez <stevechavezast@gmail.com>
Date: Sat, 2 Mar 2024 19:06:13 -0500
Subject: [PATCH 1/1] psql: fix variable existence tab completion
---
src/bin/psql/t/010_tab_completion.pl | 8 ++++++++
src/bin/psql/tab-complete.c | 4 +++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index b6575b075e..b45c39f0f5 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -413,6 +413,14 @@ check_completion(
clear_query();
+# check completion for psql variable test
+check_completion(
+ "\\echo :{?VERB\t",
+ qr/:\{\?VERBOSITY} /,
+ "complete a psql variable test");
+
+clear_query();
+
# check no-completions code path
check_completion("blarg \t\t", qr//, "check completion failure path");
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index ada711d02f..a16dac9e73 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -76,7 +76,7 @@
#endif
/* word break characters */
-#define WORD_BREAKS "\t\n@><=;|&{() "
+#define WORD_BREAKS "\t\n@><=;|&() "
/*
* Since readline doesn't let us pass any state through to the tab completion
@@ -1785,6 +1785,8 @@ psql_completion(const char *text, int start, int end)
matches = complete_from_variables(text, ":'", "'", true);
else if (text[1] == '"')
matches = complete_from_variables(text, ":\"", "\"", true);
+ else if (text[1] == '{' && text[2] == '?')
+ matches = complete_from_variables(text, ":{?", "}", true);
else
matches = complete_from_variables(text, ":", "", true);
}
--
2.40.1
On 2024-03-03 03:00 +0100, Steve Chavez wrote:
psql has the :{?name} syntax for testing a psql variable existence.
But currently doing \echo :{?VERB<Tab> doesn't trigger tab completion.
This patch fixes it. I've also included a TAP test.
Thanks. The code looks good, all tests pass, and the tab completion
works as expected when testing manually.
--
Erik
On Sun, Mar 3, 2024 at 5:37 PM Erik Wienhold <ewie@ewie.name> wrote:
On 2024-03-03 03:00 +0100, Steve Chavez wrote:
psql has the :{?name} syntax for testing a psql variable existence.
But currently doing \echo :{?VERB<Tab> doesn't trigger tab completion.
This patch fixes it. I've also included a TAP test.
Thanks. The code looks good, all tests pass, and the tab completion
works as expected when testing manually.
A nice improvement. I've checked why we have at all the '{' at
WORD_BREAKS and if we're going to break anything by removing that. It
seems that '{' here from the very beginning and it comes from the
default value of rl_basic_word_break_characters [1]. It seems that
:{?name} is the only usage of '{' sign in psql. So, removing it from
WORD_BREAKS shouldn't break anything.
I'm going to push this patch if no objections.
------
Regards,
Alexander Korotkov
Hello!
On 14.03.2024 17:57, Alexander Korotkov wrote:
On Sun, Mar 3, 2024 at 5:37 PM Erik Wienhold <ewie@ewie.name> wrote:
On 2024-03-03 03:00 +0100, Steve Chavez wrote:
psql has the :{?name} syntax for testing a psql variable existence.
But currently doing \echo :{?VERB<Tab> doesn't trigger tab completion.
This patch fixes it. I've also included a TAP test.
Thanks. The code looks good, all tests pass, and the tab completion
works as expected when testing manually.
I'm not sure if Debian 10 is actual for the current master. But, if this is the case,
i suggest a patch, since the test will not work under this OS.
The thing is that, Debian 10 will backslash curly braces and the question mark and
TAB completion will lead to the string like that:
\echo :\{\?VERBOSITY\}
instead of expected:
\echo :{?VERBOSITY}
The patch attached fix the 010_tab_completion.pl test in the same way like [1]/messages/by-id/960764.1643751011@sss.pgh.pa.us.
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
0001-Fix-variable-tab-completion-for-broken-libedit.patchtext/x-patch; charset=UTF-8; name=0001-Fix-variable-tab-completion-for-broken-libedit.patchDownload
From 455bec0d785b0f5057fc7e91a5fede458cf8fd36 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melnikov@postgrespro.ru>
Date: Mon, 6 May 2024 08:40:45 +0300
Subject: [PATCH] Fix variable tab completion for broken libedit
---
src/bin/psql/t/010_tab_completion.pl | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index b45c39f0f5..358478e935 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -414,12 +414,15 @@ check_completion(
clear_query();
# check completion for psql variable test
+# note: broken versions of libedit want to backslash curly braces and the question mark;
+# not much we can do about that
check_completion(
"\\echo :{?VERB\t",
- qr/:\{\?VERBOSITY} /,
+ qr/:\\?\{\\?\?VERBOSITY\\?} /,
"complete a psql variable test");
-clear_query();
+# broken versions of libedit require clear_line not clear_query here
+clear_line();
# check no-completions code path
check_completion("blarg \t\t", qr//, "check completion failure path");
--
2.43.2
Hi, Anton!
On Mon, May 6, 2024 at 9:05 AM Anton A. Melnikov
<a.melnikov@postgrespro.ru> wrote:
On 14.03.2024 17:57, Alexander Korotkov wrote:
On Sun, Mar 3, 2024 at 5:37 PM Erik Wienhold <ewie@ewie.name> wrote:
On 2024-03-03 03:00 +0100, Steve Chavez wrote:
psql has the :{?name} syntax for testing a psql variable existence.
But currently doing \echo :{?VERB<Tab> doesn't trigger tab completion.
This patch fixes it. I've also included a TAP test.
Thanks. The code looks good, all tests pass, and the tab completion
works as expected when testing manually.I'm not sure if Debian 10 is actual for the current master. But, if this is the case,
i suggest a patch, since the test will not work under this OS.
The thing is that, Debian 10 will backslash curly braces and the question mark and
TAB completion will lead to the string like that:\echo :\{\?VERBOSITY\}
instead of expected:
\echo :{?VERBOSITY}
The patch attached fix the 010_tab_completion.pl test in the same way like [1].
Thank you for the fix. As I get, the fix teaches
010_tab_completion.pl to tolerate the invalid result of tab
completion. Do you think we could fix it another way to make the
result of tab completion correct?
------
Regards,
Alexander Korotkov
Supabase
Hi, Alexander!
On 06.05.2024 13:19, Alexander Korotkov wrote:
The patch attached fix the 010_tab_completion.pl test in the same way like [1].
Thank you for the fix. As I get, the fix teaches
010_tab_completion.pl to tolerate the invalid result of tab
completion. Do you think we could fix it another way to make the
result of tab completion correct?
Right now i don't see any straight way to fix this to the correct tab completion.
There are several similar cases in this test.
E.g., for such a commands:
CREATE TABLE "mixedName" (f1 int, f2 text);
select * from "mi<TAB> ;
gives with debian 10:
postgres=# select * from \"mixedName\" ;
resulting in an error.
Now there is a similar workaround in the 010_tab_completion.pl with regex: qr/"mixedName\\?" /
I think if there were or will be complaints from users about this behavior in Debian 10,
then it makes sense to look for more complex solutions that will fix a backslash substitutions.
If no such complaints, then it is better to make a workaround in test.
With the best wishes,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
"Anton A. Melnikov" <a.melnikov@postgrespro.ru> writes:
On 06.05.2024 13:19, Alexander Korotkov wrote:
Now there is a similar workaround in the 010_tab_completion.pl with regex: qr/"mixedName\\?" /
I think if there were or will be complaints from users about this behavior in Debian 10,
then it makes sense to look for more complex solutions that will fix a backslash substitutions.
If no such complaints, then it is better to make a workaround in test.
Actually, I think we ought to just reject this change. Debian 10
will be two years past EOL before PG 17 ships. So I don't see a
reason to support it in the tests anymore. One of the points of
such testing is to expose broken platforms, not mask them.
Obviously, if anyone can point to a still-in-support platform
with the same bug, that calculus might change.
With respect to the other hacks Alexander mentions, maybe we
could clean some of those out too? I don't recall what platform
we had in mind there, but we've moved our goalposts on what
we support pretty far in the last couple years.
regards, tom lane
On 19.07.2024 01:10, Tom Lane wrote:
Actually, I think we ought to just reject this change. Debian 10
will be two years past EOL before PG 17 ships. So I don't see a
reason to support it in the tests anymore. One of the points of
such testing is to expose broken platforms, not mask them.Obviously, if anyone can point to a still-in-support platform
with the same bug, that calculus might change.
The bug when broken version of libedit want to backslash some symbols
(e.g. double quotas, curly braces, the question mark)
i only encountered on Debian 10 (buster).
If anyone has encountered a similar error on some other system,
please share such information.
With respect to the other hacks Alexander mentions, maybe we
could clean some of those out too? I don't recall what platform
we had in mind there, but we've moved our goalposts on what
we support pretty far in the last couple years.
Agreed that no reason to save workarounds for non-supported systems.
Here is the patch that removes fixes for Buster bug mentioned above.
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
v1-0001-Remove-workarounds-for-Debian-10.patchtext/x-patch; charset=UTF-8; name=v1-0001-Remove-workarounds-for-Debian-10.patchDownload
From 971184c49cf2c5932bb682ee977a6fef9ba4eba5 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melnikov@postgrespro.ru>
Date: Sun, 21 Jul 2024 01:37:03 +0300
Subject: [PATCH] Remove workarounds for Debian 10 (Buster) with broken
libedit from the 010_tab_completion.pl test.
---
src/bin/psql/t/010_tab_completion.pl | 27 ++++++++-------------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index b45c39f0f5..678a0f7a2c 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -167,26 +167,18 @@ check_completion(
qr/"mytab123" +"mytab246"/,
"offer multiple quoted table choices");
-# note: broken versions of libedit want to backslash the closing quote;
-# not much we can do about that
-check_completion("2\t", qr/246\\?" /,
+check_completion("2\t", qr/246" /,
"finish completion of one of multiple quoted table choices");
-# note: broken versions of libedit may leave us in a state where psql
-# thinks there's an unclosed double quote, so that we have to use
-# clear_line not clear_query here
-clear_line();
+clear_query();
# check handling of mixed-case names
-# note: broken versions of libedit want to backslash the closing quote;
-# not much we can do about that
check_completion(
"select * from \"mi\t",
- qr/"mixedName\\?" /,
+ qr/"mixedName" /,
"complete a mixed-case name");
-# as above, must use clear_line not clear_query here
-clear_line();
+clear_query();
# check case folding
check_completion("select * from TAB\t", qr/tab1 /, "automatically fold case");
@@ -198,8 +190,7 @@ clear_query();
# differently, so just check that the replacement comes out correctly
check_completion("\\DRD\t", qr/drds /, "complete \\DRD<tab> to \\drds");
-# broken versions of libedit require clear_line not clear_query here
-clear_line();
+clear_query();
# check completion of a schema-qualified name
check_completion("select * from pub\t",
@@ -261,18 +252,16 @@ check_completion(
qr|tab_comp_dir/af\a?ile|,
"filename completion with multiple possibilities");
-# broken versions of libedit require clear_line not clear_query here
+# here we inside a string literal 'afile*' and wait for a sub-choice or second TAB. So use clear_line().
clear_line();
# COPY requires quoting
-# note: broken versions of libedit want to backslash the closing quote;
-# not much we can do about that
check_completion(
"COPY foo FROM tab_comp_dir/some\t",
- qr|'tab_comp_dir/somefile\\?' |,
+ qr|'tab_comp_dir/somefile' |,
"quoted filename completion with one possibility");
-clear_line();
+clear_query();
check_completion(
"COPY foo FROM tab_comp_dir/af\t",
--
2.45.2
"Anton A. Melnikov" <a.melnikov@postgrespro.ru> writes:
On 19.07.2024 01:10, Tom Lane wrote:
With respect to the other hacks Alexander mentions, maybe we
could clean some of those out too? I don't recall what platform
we had in mind there, but we've moved our goalposts on what
we support pretty far in the last couple years.
Agreed that no reason to save workarounds for non-supported systems.
Here is the patch that removes fixes for Buster bug mentioned above.
Pushed. I shall now watch the buildfarm from a safe distance.
regards, tom lane
On 04.09.2024 23:26, Tom Lane wrote:
Pushed. I shall now watch the buildfarm from a safe distance.
Thanks! I'll be ready to fix possible falls.
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company