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


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to