config.mak.uname: add HAVE_DEV_TTY to cygwin config section

Commit Message

Ramsay Jones Sept. 9, 2024, 1:23 a.m. UTC

If neither HAVE_DEV_TTY nor GIT_WINDOWS_NATIVE is set, while compiling
the 'compat/terminal.c' code, then the fallback code calls the system
getpass() function. Unfortunately, this ignores the 'echo' parameter of
the git_terminal_prompt() function, since it has no way to implement that
functionality. This results in a less than optimal user experience on
cygwin, which does not define either of those build flags.

However, cygwin does have a functional '/dev/tty', so that it can build
with HAVE_DEV_TTY and benefit from the improved user experience.

The improved git_terminal_prompt() function that comes with HAVE_DEV_TTY
is used in the git_prompt() function, which in turn is used by the
'git credential', 'git bisect' and 'git help' commands. In addition to
git_terminal_prompt(), read_key_without_echo() is likewise improved and
used by the 'git add -p' command.

While using the 'git credential fill' command, for example:

  $ printf "%s\n" protocol=https host=example.com path=git | ./git credential fill
  Username for 'https://example.com': user
  Password for 'https://[e-mail]':
  protocol=https
  host=example.com
  username=user
  password=pass
  $

The 'user' name is now echoed while typing (the password isn't), where this
wasn't the case before.

When using the auto-correct feature:

  $ ./git -c help.autocorrect=prompt fred
  WARNING: You called a Git command named 'fred', which does not exist.
  Run 'grep' instead [y/N]? n
  $ ./git -c help.autocorrect=prompt fred
  WARNING: You called a Git command named 'fred', which does not exist.
  Run 'grep' instead [y/N]? y
  fatal: no pattern given
  $

The user can actually see what they are typing at the prompt. Similar
comments apply to 'git bisect':

  $ ./git bisect bad master~1
  You need to start by "git bisect start"

  Do you want me to do it for you [Y/n]? y
  status: waiting for both good and bad commits
  status: waiting for good commit(s), bad commit known
  $ ./git bisect reset
  Already on 'master-tmp'
  $

  $ ./git bisect start
  status: waiting for both good and bad commits
  $ ./git bisect bad master~1
  status: waiting for good commit(s), bad commit known
  $ ./git bisect next
  warning: bisecting only with a bad commit
  Are you sure [Y/n]? n
  $ ./git bisect reset
  Already on 'master-tmp'
  $

The read_key_without_echo() function leads to a much improved 'git add -p'
command, when the 'interactive.singleKey' configuration is set:

  $ cd ..
  $ mkdir test-git
  $ cd test-git
  $ git init -q
  $ echo foo >file
  $ git add file
  $ echo bar >file
  $ ../git/git -c interactive.singleKey=true add -p
  diff --git a/file b/file
  index 257cc56..5716ca5 100644
  --- a/file
  +++ b/file
  @@ -1 +1 @@
  -foo
  +bar
  (1/1) Stage this hunk [y,n,q,a,d,e,p,?]? y

  $

Note that, not only is the user input echoed, but that it is immediately
accepted (without having to type <return>) and the program exits with the
hunk staged (in this case) or not.

In order to reap these benefits, set the HAVE_DEV_TTY build flag in the
cygwin configuration section of config.mak.uname.

Signed-off-by: Ramsay Jones <[e-mail]>
---

Hi Junio,

After more testing, I removed the RFC from this patch and actually wrote
a commit message. (I wasn't sure if I should mark this with v2 as well?).

I was a bit surprised that this went unnoticed for so long, but I don't
use 'git credential' (shh-agent is all I need), 'git add -p' (vim works
for me!) or used the help.autocorrect=prompt. I have used 'git bisect'
many, many times (of course), but I don't recall ever seeing either of
those prompts! This goes for Linux as well as cygwin. :)

ATB,
Ramsay Jones

 config.mak.uname | 1 +
 1 file changed, 1 insertion(+)

Comments

Jeff King Sept. 9, 2024, 7:08 a.m. UTC | #1

On Mon, Sep 09, 2024 at 02:23:48AM +0100, Ramsay Jones wrote:

> After more testing, I removed the RFC from this patch and actually wrote
> a commit message. (I wasn't sure if I should mark this with v2 as well?).
> 
> I was a bit surprised that this went unnoticed for so long, but I don't
> use 'git credential' (shh-agent is all I need), 'git add -p' (vim works
> for me!) or used the help.autocorrect=prompt. I have used 'git bisect'
> many, many times (of course), but I don't recall ever seeing either of
> those prompts! This goes for Linux as well as cygwin. :)

The patch unsurprisingly looks good to me.

I think most of what you are describing in the commit message is all the
normal and expected benefits of HAVE_DEV_TTY. :) But I don't mind erring
on the side of over-explaining there.

-Peff

Patch

diff --git a/config.mak.uname b/config.mak.uname
index 904bcf3598..d5112168a4 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -248,6 +248,7 @@  ifeq ($(uname_O),Cygwin)
         else
 		NO_REGEX = UnfortunatelyYes
         endif
+	HAVE_DEV_TTY = YesPlease
 	HAVE_ALLOCA_H = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes