Thread: 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
Attachment
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.
Links.
1. https://tiswww.case.edu/php/chet/readline/readline.html#index-rl_005fbasic_005fword_005fbreak_005fcharacters
------
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].
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
[1] https://www.postgresql.org/message-id/960764.1643751011@sss.pgh.pa.us
Attachment
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
Attachment
"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