[PATCH/RFC] compat/terminal: support echoing on windows

[PATCH/RFC] compat/terminal: support echoing on windows

[Erik Faye-Lund]

Without /dev/tty support, git_terminal_prompt simply ignores the
'echo'-parameter. On Windows we can do better by clevering up our
getpass-implementation a bit so it can conditionally echo.

While we're at it, plug a small memory-leak by returning a pointer
to a static strbuf instead of detaching it. This is the same thing
the /dev/tty-version of git_terminal_prompt does, and the callee
doesn't expect to have to free it's memory.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/mingw.c    | 15 ---------------
 compat/mingw.h    |  2 --
 compat/terminal.c | 20 ++++++++++++++++++++
 3 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index afc892d..56ab74c 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1699,21 +1699,6 @@ int link(const char *oldpath, const char *newpath)
 	return 0;
 }
 
-char *getpass(const char *prompt)
-{
-	struct strbuf buf = STRBUF_INIT;
-
-	fputs(prompt, stderr);
-	for (;;) {
-		char c = _getch();
-		if (c == '\r' || c == '\n')
-			break;
-		strbuf_addch(&buf, c);
-	}
-	fputs("\n", stderr);
-	return strbuf_detach(&buf, NULL);
-}
-
 pid_t waitpid(pid_t pid, int *status, int options)
 {
 	HANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
diff --git a/compat/mingw.h b/compat/mingw.h
index 61a6521..5e64a98 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -55,8 +55,6 @@ struct passwd {
 	char *pw_dir;
 };
 
-extern char *getpass(const char *prompt);
-
 typedef void (__cdecl *sig_handler_t)(int);
 struct sigaction {
 	sig_handler_t sa_handler;
diff --git a/compat/terminal.c b/compat/terminal.c
index 6d16c8f..53c5166 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -71,6 +71,26 @@ char *git_terminal_prompt(const char *prompt, int echo)
 	return buf.buf;
 }
 
+#elif defined(WIN32)
+
+char *git_terminal_prompt(const char *prompt, int echo)
+{
+	static struct strbuf buf = STRBUF_INIT;
+
+	fputs(prompt, stderr);
+	strbuf_reset(&buf);
+	for (;;) {
+		int c = _getch();
+		if (c == '\n' || c == '\r')
+			break;
+		if (echo)
+			putc(c, stderr);
+		strbuf_addch(&buf, c);
+	}
+	putc('\n', stderr);
+	return buf.buf;
+}
+
 #else
 
 char *git_terminal_prompt(const char *prompt, int echo)
-- 
1.7.11.1.27.gdae0dbb

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

Re: [PATCH/RFC] compat/terminal: support echoing on windows

[Johannes Schindelin]

Hi,

On Sun, 24 Jun 2012, Erik Faye-Lund wrote:

> Without /dev/tty support, git_terminal_prompt simply ignores the
> 'echo'-parameter. On Windows we can do better by clevering up our
> getpass-implementation a bit so it can conditionally echo.
> 
> While we're at it, plug a small memory-leak by returning a pointer
> to a static strbuf instead of detaching it. This is the same thing
> the /dev/tty-version of git_terminal_prompt does, and the callee
> doesn't expect to have to free it's memory.

Looks good, please apply!

Ciao,
Dscho

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

Re: [PATCH/RFC] compat/terminal: support echoing on windows

[Erik Faye-Lund]

On Sun, Jun 24, 2012 at 4:38 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sun, 24 Jun 2012, Erik Faye-Lund wrote:
>
>> Without /dev/tty support, git_terminal_prompt simply ignores the
>> 'echo'-parameter. On Windows we can do better by clevering up our
>> getpass-implementation a bit so it can conditionally echo.
>>
>> While we're at it, plug a small memory-leak by returning a pointer
>> to a static strbuf instead of detaching it. This is the same thing
>> the /dev/tty-version of git_terminal_prompt does, and the callee
>> doesn't expect to have to free it's memory.
>
> Looks good, please apply!

Done.

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

Re: [PATCH/RFC] compat/terminal: support echoing on windows

[Erik Faye-Lund]

On Sun, Jun 24, 2012 at 3:35 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> Without /dev/tty support, git_terminal_prompt simply ignores the
> 'echo'-parameter. On Windows we can do better by clevering up our
> getpass-implementation a bit so it can conditionally echo.
>
> While we're at it, plug a small memory-leak by returning a pointer
> to a static strbuf instead of detaching it. This is the same thing
> the /dev/tty-version of git_terminal_prompt does, and the callee
> doesn't expect to have to free it's memory.
>
> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> ---
>  compat/mingw.c    | 15 ---------------
>  compat/mingw.h    |  2 --
>  compat/terminal.c | 20 ++++++++++++++++++++
>  3 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index afc892d..56ab74c 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1699,21 +1699,6 @@ int link(const char *oldpath, const char *newpath)
>        return 0;
>  }
>
> -char *getpass(const char *prompt)
> -{
> -       struct strbuf buf = STRBUF_INIT;
> -
> -       fputs(prompt, stderr);
> -       for (;;) {
> -               char c = _getch();
> -               if (c == '\r' || c == '\n')
> -                       break;
> -               strbuf_addch(&buf, c);
> -       }
> -       fputs("\n", stderr);
> -       return strbuf_detach(&buf, NULL);
> -}
> -
>  pid_t waitpid(pid_t pid, int *status, int options)
>  {
>        HANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 61a6521..5e64a98 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -55,8 +55,6 @@ struct passwd {
>        char *pw_dir;
>  };
>
> -extern char *getpass(const char *prompt);
> -
>  typedef void (__cdecl *sig_handler_t)(int);
>  struct sigaction {
>        sig_handler_t sa_handler;
> diff --git a/compat/terminal.c b/compat/terminal.c
> index 6d16c8f..53c5166 100644
> --- a/compat/terminal.c
> +++ b/compat/terminal.c
> @@ -71,6 +71,26 @@ char *git_terminal_prompt(const char *prompt, int echo)
>        return buf.buf;
>  }
>
> +#elif defined(WIN32)
> +
> +char *git_terminal_prompt(const char *prompt, int echo)
> +{
> +       static struct strbuf buf = STRBUF_INIT;
> +
> +       fputs(prompt, stderr);
> +       strbuf_reset(&buf);
> +       for (;;) {
> +               int c = _getch();
> +               if (c == '\n' || c == '\r')
> +                       break;
> +               if (echo)
> +                       putc(c, stderr);
> +               strbuf_addch(&buf, c);
> +       }
> +       putc('\n', stderr);
> +       return buf.buf;
> +}
> +
>  #else
>
>  char *git_terminal_prompt(const char *prompt, int echo)
> --
> 1.7.11.1.27.gdae0dbb
>

Turns out, this patch is incorrect; _getch is defined in conio.h,
which is only included in compat/mingw.c and compat/msvc.c.

This patch on top fixes it:

diff --git a/compat/mingw.c b/compat/mingw.c
index 56ab74c..d7d4aea 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1,6 +1,5 @@
 #include "../git-compat-util.h"
 #include "win32.h"
-#include <conio.h>
 #include "../strbuf.h"
 #include "../run-command.h"

diff --git a/compat/msvc.c b/compat/msvc.c
index 71843d7..a3a4e0e 100644
--- a/compat/msvc.c
+++ b/compat/msvc.c
@@ -1,6 +1,5 @@
 #include "../git-compat-util.h"
 #include "win32.h"
-#include <conio.h>
 #include "../strbuf.h"

 #include "mingw.c"
diff --git a/git-compat-util.h b/git-compat-util.h
index 5bd9ad7..fa3e1a9 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -88,6 +88,7 @@
 #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
 #include <winsock2.h>
 #include <windows.h>
+#include <conio.h>
 #endif

 #include <unistd.h>

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

Re: [PATCH/RFC] compat/terminal: support echoing on windows

[Jeff King]

On Sun, Jun 24, 2012 at 03:35:03PM +0200, Erik Faye-Lund wrote:

> Without /dev/tty support, git_terminal_prompt simply ignores the
> 'echo'-parameter. On Windows we can do better by clevering up our
> getpass-implementation a bit so it can conditionally echo.
> 
> While we're at it, plug a small memory-leak by returning a pointer
> to a static strbuf instead of detaching it. This is the same thing
> the /dev/tty-version of git_terminal_prompt does, and the callee
> doesn't expect to have to free it's memory.

Very nice. I was hoping for something exactly like this when I wrote the
original git_terminal_prompt patch.

-Peff

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en