Re: BUG #6510: A simple prompt is displayed using wrong charset

On Sun, Oct 14, 2012 at 10:35:04AM +0400, Alexander Law wrote:
> I agree with you, CONOUT$ way is much simpler. Please look at the patch.

See comments below.

> Regarding msys - yes, that check was not correct.
> In fact you can use "con" with msys, if you run sh.exe, not a graphical
> terminal.
> So the issue with con not related to msys, but to some terminal
> implementations.
> Namely, I see that con is not supported by rxvt, mintty and xterm (from
> x.cygwin project).
> (rxvt was default terminal for msys 1.0.10, so I think such behavior was
> considered as msys feature because of this)
> Your solution to use IsWindowVisible(GetConsoleWindow()) works for these
> terminals (I've made simple test and it returns false for all of them),
> but this check will not work for telnet (console app running through
> telnet can use con/conout).

Thanks for testing those environments. I can reproduce the distinctive
behavior when a Windows telnet client connects to a Windows telnet server.
When I connect to a Windows telnet server from a GNU/Linux system, I get the
normal invisible-console behavior.

I also get the invisible-console behavior in PowerShell ISE.

> Maybe this should be considered as a distinct bug with another patch
> required? (I see no ideal solution for it yet. Probably it's possible to
> detect not "ostype", but these terminals, though it would not be generic
> too.)

Using stdin/stderr when we could have used the console is a mild loss; use
cases involving redirected output will need to account for the abnormality.
Interacting with a user-invisible console is a large loss; prompts will hang
indefinitely. Therefore, the test should err on the side of stdin/stderr.

Since any change here seems to have its own trade-offs, yes, let's leave it
for a separate patch.

> And there is another issue with a console charset. When writing string
> to a console CRT converts it to console encoding, but when reading input
> back it doesn't. So it seems, there should be conversion from
> ConsoleCP() to ACP() and then probably to UTF-8 to make postgres
> utilities support national chars in passwords or usernames (with
> createuser --interactive).

Yes, that also deserves attention. I do not know whether converting to UTF-8
is correct. Given a username <foo> containing non-ASCII characters, you
should be able to input <foo> the same way for both "psql -U <foo>" and the
createuser prompt. We should also be thoughtful about backward compatibility.

> I think it can be fixed as another bug too.

Agreed.

> --- a/src/port/sprompt.c
> +++ b/src/port/sprompt.c
> @@ -60,8 +60,13 @@ simple_prompt(const char *prompt, int maxlen, bool echo)
> * Do not try to collapse these into one "w+" mode file. Doesn't work on
> * some platforms (eg, HPUX 10.20).
> */
> +#ifdef WIN32
> + termin = fopen("CONIN$", "r");
> + termout = fopen("CONOUT$", "w+");

This definitely needs a block comment explaining the behaviors that led us to
select this particular implementation.

> +#else
> termin = fopen(DEVTTY, "r");
> termout = fopen(DEVTTY, "w");

This thread has illustrated that the DEVTTY abstraction does not suffice. I
think we should remove it entirely. Remove it from port.h; use literal
"/dev/tty" here; re-add it as a local #define near the one remaining use, with
an XXX comment indicating that the usage is broken.

If it would help, I can prepare a version with the comment changes and
refactoring I have in mind.

> +#endif
> if (!termin || !termout
> #ifdef WIN32
> /* See DEVTTY comment for msys */

Thanks,
nm