Windows v readline

Started by Andrew Dunstanover 6 years ago14 messages
#1Andrew Dunstan
andrew.dunstan@2ndquadrant.com

The configure code currently has this:

# readline on MinGW has problems with backslashes in psql and other bugs.
# This is particularly a problem with non-US code pages.
# Therefore disable its use until we understand the cause. 2004-07-20
if test "$PORTNAME" = "win32"; then
  if test "$with_readline" = yes; then
    AC_MSG_WARN([*** Readline does not work on MinGW --- disabling])
    with_readline=no
  fi
fi

2004 is a very long time ago. Has anyone looked at this more recently?
It would certainly be nice to have readline-enabled psql on Windows if
possible.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#2Victor Spirin
v.spirin@postgrespro.ru
In reply to: Andrew Dunstan (#1)
1 attachment(s)
psql tab-complete

I found some problem with tab-complete in the 12 version.  I checked
this in the Windows.

Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

Attachments:

tab-complete.patchtext/plain; charset=UTF-8; name=tab-complete.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index e00dbab5aa..5d7f24e57a 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3577,7 +3577,7 @@ psql_completion(const char *text, int start, int end)
 
 /* WHERE */
 	/* Simple case of the word before the where being the table name */
-	else if (TailMatches(MatchAny, "WHERE"))
+	else if (TailMatches("WHERE", MatchAny))
 		COMPLETE_WITH_ATTR(prev2_wd, "");
 
 /* ... FROM ... */
#3Victor Spirin
v.spirin@postgrespro.ru
In reply to: Victor Spirin (#2)
Re: psql tab-complete

Sorry for wrong place and contents of my message.

It seems that the VA_ARGS_NARGS (__ VA_ARGS__) macros always return 1 on
Windows.

Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

24.10.2019 19:11, Victor Spirin пишет:

Show quoted text

I found some problem with tab-complete in the 12 version.  I checked
this in the Windows.

Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Victor Spirin (#2)
Re: psql tab-complete

Victor Spirin <v.spirin@postgrespro.ru> writes:

I found some problem with tab-complete in the 12 version.  I checked
this in the Windows.

This change seems to break the case intended by the comment,
ie given the context

SELECT * FROM tablename WHERE <tab>

we want to offer the columns of "tablename" as completions.

I'd be the first to agree that that's completely lame, as there
are any number of related cases it fails to cover ... but this
patch isn't making it better.

regards, tom lane

#5Victor Spirin
v.spirin@postgrespro.ru
In reply to: Tom Lane (#4)
Re: psql tab-complete

Yes, I found, that VA_ARGS_NARGS(__ VA_ARGS__) macros always return 1 on
Windows.

Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

25.10.2019 0:48, Tom Lane пишет:

Show quoted text

Victor Spirin <v.spirin@postgrespro.ru> writes:

I found some problem with tab-complete in the 12 version.  I checked
this in the Windows.

This change seems to break the case intended by the comment,
ie given the context

SELECT * FROM tablename WHERE <tab>

we want to offer the columns of "tablename" as completions.

I'd be the first to agree that that's completely lame, as there
are any number of related cases it fails to cover ... but this
patch isn't making it better.

regards, tom lane

#6Victor Spirin
v.spirin@postgrespro.ru
In reply to: Victor Spirin (#5)
1 attachment(s)
Re: psql tab-complete

This patch resolved one problem in the tab-complete.c on MSVC. The
VA_ARGS_NARGS macros now work correctly on Windows.

Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

25.10.2019 0:53, Victor Spirin пишет:

Show quoted text

Yes, I found, that VA_ARGS_NARGS(__ VA_ARGS__) macros always return 1
on Windows.

Victor Spirin
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company

25.10.2019 0:48, Tom Lane пишет:

Victor Spirin <v.spirin@postgrespro.ru> writes:

I found some problem with tab-complete in the 12 version.  I checked
this in the Windows.

This change seems to break the case intended by the comment,
ie given the context

    SELECT * FROM tablename WHERE <tab>

we want to offer the columns of "tablename" as completions.

I'd be the first to agree that that's completely lame, as there
are any number of related cases it fails to cover ... but this
patch isn't making it better.

            regards, tom lane

Attachments:

nargs.patchtext/plain; charset=UTF-8; name=nargs.patchDownload
diff --git a/src/include/c.h b/src/include/c.h
index edc7822b0f..ed7099164b 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -238,6 +238,19 @@
  * the call so that that is the appropriate one of the list of constants.
  * This idea is due to Laurent Deniau.
  */
+#ifdef _MSC_VER
+#define EXPAND(args) args
+#define VA_ARGS_NARGS(...) \
+	VA_ARGS_NARGS_ EXPAND((__VA_ARGS__, \
+				   63,62,61,60,                   \
+				   59,58,57,56,55,54,53,52,51,50, \
+				   49,48,47,46,45,44,43,42,41,40, \
+				   39,38,37,36,35,34,33,32,31,30, \
+				   29,28,27,26,25,24,23,22,21,20, \
+				   19,18,17,16,15,14,13,12,11,10, \
+				   9, 8, 7, 6, 5, 4, 3, 2, 1, 0))
+#else
+
 #define VA_ARGS_NARGS(...) \
 	VA_ARGS_NARGS_(__VA_ARGS__, \
 				   63,62,61,60,                   \
@@ -247,6 +260,8 @@
 				   29,28,27,26,25,24,23,22,21,20, \
 				   19,18,17,16,15,14,13,12,11,10, \
 				   9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+#endif
+
 #define VA_ARGS_NARGS_( \
 	_01,_02,_03,_04,_05,_06,_07,_08,_09,_10, \
 	_11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \
#7Michael Paquier
michael@paquier.xyz
In reply to: Victor Spirin (#6)
Re: psql tab-complete

On Fri, Oct 25, 2019 at 11:57:18AM +0300, Victor Spirin wrote:

This patch resolved one problem in the tab-complete.c on MSVC. The
VA_ARGS_NARGS macros now work correctly on Windows.

Can you explain why and in what the use of EXPAND() helps with MSVC
builds? Any references which help to understand why this is better?
If this change is needed, this also surely needs a comment to explain
the difference.
--
Michael

#8Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Andrew Dunstan (#1)
Re: Windows v readline

On 2019-09-29 22:55, Andrew Dunstan wrote:

The configure code currently has this:

# readline on MinGW has problems with backslashes in psql and other bugs.
# This is particularly a problem with non-US code pages.
# Therefore disable its use until we understand the cause. 2004-07-20
if test "$PORTNAME" = "win32"; then
  if test "$with_readline" = yes; then
    AC_MSG_WARN([*** Readline does not work on MinGW --- disabling])
    with_readline=no
  fi
fi

2004 is a very long time ago. Has anyone looked at this more recently?
It would certainly be nice to have readline-enabled psql on Windows if
possible.

I tried this out. First, it doesn't build, because readline doesn't do
the dllimport/dllexport dance on global variables, so all references to
rl_* global variables in tab-complete.c fail (similar to [0]/messages/by-id/001101c3eb6a$3b275500$f800a8c0@kuczek.pl). After
patching those out, it builds, but it doesn't work. It doesn't print a
prompt, keys don't do anything sensible. I can enter SQL commands and
get results back, but the readline part doesn't do anything sensible AFAICT.

Perhaps I did something wrong. You can still use readline without
global variables, but it seems like a serious restriction and makes me
wonder whether this has actually ever been used before. It's curious
that MSYS2 ships a readline build for mingw. Is there other software
that uses it on Windows?

[0]: /messages/by-id/001101c3eb6a$3b275500$f800a8c0@kuczek.pl
/messages/by-id/001101c3eb6a$3b275500$f800a8c0@kuczek.pl

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#8)
Re: Windows v readline

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-09-29 22:55, Andrew Dunstan wrote:

It would certainly be nice to have readline-enabled psql on Windows if
possible.

I tried this out. First, it doesn't build, because readline doesn't do
the dllimport/dllexport dance on global variables, so all references to
rl_* global variables in tab-complete.c fail (similar to [0]). After
patching those out, it builds, but it doesn't work.

What do you mean by "patching those out" --- you removed all of
tab-complete's external variable assignments? That would certainly
disable tab completion, at a minimum.

It doesn't print a
prompt, keys don't do anything sensible. I can enter SQL commands and
get results back, but the readline part doesn't do anything sensible AFAICT.

I wonder if readline was confused about the terminal type, or if it
decided that the input file was not-a-tty for some reason.

regards, tom lane

#10Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#9)
Re: Windows v readline

On 2019-12-30 14:28, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-09-29 22:55, Andrew Dunstan wrote:

It would certainly be nice to have readline-enabled psql on Windows if
possible.

I tried this out. First, it doesn't build, because readline doesn't do
the dllimport/dllexport dance on global variables, so all references to
rl_* global variables in tab-complete.c fail (similar to [0]). After
patching those out, it builds, but it doesn't work.

What do you mean by "patching those out" --- you removed all of
tab-complete's external variable assignments? That would certainly
disable tab completion, at a minimum.

Yeah, basically remove tab completion altogether. But readline would
still be useful without that for line editing and history.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#11Thomas Munro
thomas.munro@gmail.com
In reply to: Michael Paquier (#7)
Re: psql tab-complete

On Sat, Oct 26, 2019 at 4:59 PM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Oct 25, 2019 at 11:57:18AM +0300, Victor Spirin wrote:

This patch resolved one problem in the tab-complete.c on MSVC. The
VA_ARGS_NARGS macros now work correctly on Windows.

Can you explain why and in what the use of EXPAND() helps with MSVC
builds? Any references which help to understand why this is better?
If this change is needed, this also surely needs a comment to explain
the difference.

Since I really want to be able to use VA_ARGS_NARGS() elsewhere, I
looked into this. There are various derivatives of that macro, some
using GCC/Clang-only syntax and that work on GCC and MSVC, splattered
all over the internet, but the original, coming as it does from a C
standards newsgroup[1]https://groups.google.com/g/comp.std.c/c/d-6Mj5Lko_s, does not. There are also lots of complaints
that the original standard version doesn't work on MSVC, with
analysis:

https://stackoverflow.com/questions/5134523/msvc-doesnt-expand-va-args-correctly
https://stackoverflow.com/questions/32399191/va-args-expansion-using-msvc
https://learn.microsoft.com/en-us/cpp/build/reference/zc-preprocessor?view=msvc-170

The short version is that __VA_ARGS__ is not tokenized the way the
standard requires (it's considered to be a single token unless you
shove it back through the preprocessor again, which is what EXPAND()
does), but you can fix that with /Zc:preprocessor. That switch only
works in Visual Studio 2019 and up, and maybe also 2017 if you spell
it /experimental:preprocessor. We still claim to support older
compilers. Assuming those switches actually work as claimed, I see
two choices: commit this hack with a comment reminding us to clean it
up later, or drop 2015.

[1]: https://groups.google.com/g/comp.std.c/c/d-6Mj5Lko_s

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Munro (#11)
Re: psql tab-complete

Thomas Munro <thomas.munro@gmail.com> writes:

Since I really want to be able to use VA_ARGS_NARGS() elsewhere,

+1, that'd be handy.

... Assuming those switches actually work as claimed, I see
two choices: commit this hack with a comment reminding us to clean it
up later, or drop 2015.

As long as we can hide the messiness inside a macro definition,
I'd vote for the former.

regards, tom lane

#13Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#12)
Re: psql tab-complete

On Wed, Dec 21, 2022 at 05:56:05PM -0500, Tom Lane wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

Since I really want to be able to use VA_ARGS_NARGS() elsewhere,

+1, that'd be handy.

... Assuming those switches actually work as claimed, I see
two choices: commit this hack with a comment reminding us to clean it
up later, or drop 2015.

As long as we can hide the messiness inside a macro definition,
I'd vote for the former.

Agreed, even if it is worth noting that all the buildfarm members
with MSVC use 2017 or newer.
--
Michael

#14Thomas Munro
thomas.munro@gmail.com
In reply to: Michael Paquier (#13)
Re: psql tab-complete

On Thu, Dec 22, 2022 at 3:25 PM Michael Paquier <michael@paquier.xyz> wrote:

On Wed, Dec 21, 2022 at 05:56:05PM -0500, Tom Lane wrote:

Thomas Munro <thomas.munro@gmail.com> writes:

Since I really want to be able to use VA_ARGS_NARGS() elsewhere,

+1, that'd be handy.

... Assuming those switches actually work as claimed, I see
two choices: commit this hack with a comment reminding us to clean it
up later, or drop 2015.

As long as we can hide the messiness inside a macro definition,
I'd vote for the former.

Agreed, even if it is worth noting that all the buildfarm members
with MSVC use 2017 or newer.

Thanks. Pushed.

PS is it a mistake that we still mention SDK 8.1 in the docs?