Re: psql: fix variable existence tab completion

2024-05-07 Thread Anton A. Melnikov

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 ;

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




Re: psql: fix variable existence tab completion

2024-05-06 Thread Alexander Korotkov
Hi, Anton!

On Mon, May 6, 2024 at 9:05 AM Anton A. Melnikov
 wrote:
> On 14.03.2024 17:57, Alexander Korotkov wrote:
> > On Sun, Mar 3, 2024 at 5:37 PM Erik Wienhold  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 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




Re: psql: fix variable existence tab completion

2024-05-06 Thread Anton A. Melnikov

Hello!

On 14.03.2024 17:57, Alexander Korotkov wrote:

On Sun, Mar 3, 2024 at 5:37 PM Erik Wienhold  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 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.1643751...@sss.pgh.pa.usFrom 455bec0d785b0f5057fc7e91a5fede458cf8fd36 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" 
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



Re: psql: fix variable existence tab completion

2024-03-14 Thread Alexander Korotkov
On Sun, Mar 3, 2024 at 5:37 PM Erik Wienhold  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 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




Re: psql: fix variable existence tab completion

2024-03-03 Thread Erik Wienhold
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 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




psql: fix variable existence tab completion

2024-03-02 Thread Steve Chavez
Hello hackers,

psql has the :{?name} syntax for testing a psql variable existence.

But currently doing \echo :{?VERB doesn't trigger tab completion.

This patch fixes it. I've also included a TAP test.

Best regards,
Steve Chavez
From adb1f997b67d8ef603017ab34e1b9061e4e229ea Mon Sep 17 00:00:00 2001
From: steve-chavez 
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