diff mbox series

[1/4] terminal: use flags for save_term()

Message ID 20220304131126.8293-2-phillip.wood123@gmail.com (mailing list archive)
State Superseded
Headers show
Series builtin add -p: hopefully final readkey fixes | expand

Commit Message

Phillip Wood March 4, 2022, 1:11 p.m. UTC
From: Phillip Wood <phillip.wood@dunelm.org.uk>

The next commit will add another flag in addition to the existing
full_duplex so change the function signature to take an unsigned flags
argument. Also alter the functions that call save_term() so that they
can pass flags down to it.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 compat/terminal.c | 41 +++++++++++++++++++++--------------------
 compat/terminal.h |  5 ++++-
 2 files changed, 25 insertions(+), 21 deletions(-)

Comments

Ramsay Jones March 4, 2022, 8:40 p.m. UTC | #1
Hi Phillip,

I have not studied/applied your patches, they are just floating
past my inbox, so please ignore me if I have misunderstood ...

On 04/03/2022 13:11, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> The next commit will add another flag in addition to the existing
> full_duplex so change the function signature to take an unsigned flags
> argument. Also alter the functions that call save_term() so that they
> can pass flags down to it.
> 
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  compat/terminal.c | 41 +++++++++++++++++++++--------------------
>  compat/terminal.h |  5 ++++-
>  2 files changed, 25 insertions(+), 21 deletions(-)
> 
> diff --git a/compat/terminal.c b/compat/terminal.c
> index d882dfa06e..bad8e04cd8 100644
> --- a/compat/terminal.c
> +++ b/compat/terminal.c
> @@ -34,7 +34,7 @@ void restore_term(void)
>  	sigchain_pop_common();
>  }
>  
> -int save_term(int full_duplex)
> +int save_term(unsigned flags)
>  {
>  	if (term_fd < 0)
>  		term_fd = open("/dev/tty", O_RDWR);
> @@ -47,11 +47,11 @@ int save_term(int full_duplex)
>  	return 0;
>  }
>  
> -static int disable_bits(tcflag_t bits)
> +static int disable_bits(unsigned flags, tcflag_t bits)

.. you add the 'flags' as the new first parameter ...

>  {
>  	struct termios t;
>  
> -	if (save_term(0) < 0)
> +	if (save_term(flags) < 0)
>  		goto error;
>  
>  	t = old_term;
> @@ -71,14 +71,14 @@ static int disable_bits(tcflag_t bits)
>  	return -1;
>  }
>  
> -static int disable_echo(void)
> +static int disable_echo(unsigned flags)
>  {
> -	return disable_bits(ECHO);
> +	return disable_bits(flags, ECHO);

.. and pass it as first parameter, good, and ...

>  }
>  
> -static int enable_non_canonical(void)
> +static int enable_non_canonical(unsigned flags)
>  {
> -	return disable_bits(ICANON | ECHO);
> +	return disable_bits(flags, ICANON | ECHO);

.. here as well, good, and ...

>  }
>  
>  #elif defined(GIT_WINDOWS_NATIVE)
> @@ -126,15 +126,15 @@ void restore_term(void)
>  	hconin = hconout = INVALID_HANDLE_VALUE;
>  }
>  
> -int save_term(int full_duplex)
> +int save_term(unsigned flags)
>  {
>  	hconin = CreateFileA("CONIN$", GENERIC_READ | GENERIC_WRITE,
>  	    FILE_SHARE_READ, NULL, OPEN_EXISTING,
>  	    FILE_ATTRIBUTE_NORMAL, NULL);
>  	if (hconin == INVALID_HANDLE_VALUE)
>  		return -1;
>  
> -	if (full_duplex) {
> +	if (flags & SAVE_TERM_DUPLEX) {
>  		hconout = CreateFileA("CONOUT$", GENERIC_READ | GENERIC_WRITE,
>  			FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
>  			FILE_ATTRIBUTE_NORMAL, NULL);
> @@ -154,7 +154,7 @@ int save_term(int full_duplex)
>  	return -1;
>  }
>  
> -static int disable_bits(DWORD bits)
> +static int disable_bits(unsigned flags, DWORD bits)

.. Huh? Ah, the DWORD suggests this is in an #ifdef'd windows
part of the file, OK. ...

>  {
>  	if (use_stty) {
>  		struct child_process cp = CHILD_PROCESS_INIT;
> @@ -191,7 +191,7 @@ static int disable_bits(DWORD bits)
>  		use_stty = 0;
>  	}
>  
> -	if (save_term(0) < 0)
> +	if (save_term(flags) < 0)
>  		return -1;
>  
>  	if (!SetConsoleMode(hconin, cmode_in & ~bits)) {
> @@ -204,14 +204,15 @@ static int disable_bits(DWORD bits)
>  	return 0;
>  }
>  
> -static int disable_echo(void)
> +static int disable_echo(unsigned flags)
>  {
> -	return disable_bits(ENABLE_ECHO_INPUT);
> +	return disable_bits(ENABLE_ECHO_INPUT, flags);

.. but here, you pass the flags as the second parameter. ;-)

>  }
>  
> -static int enable_non_canonical(void)
> +static int enable_non_canonical(unsigned flags)
>  {
> -	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);
> +	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT,
> +			    flags);

.. ditto here.

ATB,
Ramsay Jones


>  }
>  
>  /*
> @@ -267,7 +268,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
>  		return NULL;
>  	}
>  
> -	if (!echo && disable_echo()) {
> +	if (!echo && disable_echo(0)) {
>  		fclose(input_fh);
>  		fclose(output_fh);
>  		return NULL;
> @@ -361,7 +362,7 @@ int read_key_without_echo(struct strbuf *buf)
>  	static int warning_displayed;
>  	int ch;
>  
> -	if (warning_displayed || enable_non_canonical() < 0) {
> +	if (warning_displayed || enable_non_canonical(0) < 0) {
>  		if (!warning_displayed) {
>  			warning("reading single keystrokes not supported on "
>  				"this platform; reading line instead");
> @@ -413,10 +414,10 @@ int read_key_without_echo(struct strbuf *buf)
>  
>  #else
>  
> -int save_term(int full_duplex)
> +int save_term(unsigned flags)
>  {
> -	/* full_duplex == 1, but no support available */
> -	return -full_duplex;
> +	/* no duplex support available */
> +	return -!!(flags & SAVE_TERM_DUPLEX);
>  }
>  
>  void restore_term(void)
> diff --git a/compat/terminal.h b/compat/terminal.h
> index 0fb9fa147c..f24b91390d 100644
> --- a/compat/terminal.h
> +++ b/compat/terminal.h
> @@ -1,14 +1,17 @@
>  #ifndef COMPAT_TERMINAL_H
>  #define COMPAT_TERMINAL_H
>  
> +/* Save input and output settings */
> +#define SAVE_TERM_DUPLEX (1u << 0)
> +
>  /*
>   * Save the terminal attributes so they can be restored later by a
>   * call to restore_term(). Note that every successful call to
>   * save_term() must be matched by a call to restore_term() even if the
>   * attributes have not been changed. Returns 0 on success, -1 on
>   * failure.
>   */
> -int save_term(int full_duplex);
> +int save_term(unsigned flags);
>  /* Restore the terminal attributes that were saved with save_term() */
>  void restore_term(void);
>
Ævar Arnfjörð Bjarmason March 5, 2022, 2:02 p.m. UTC | #2
On Fri, Mar 04 2022, Phillip Wood wrote:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> The next commit will add another flag in addition to the existing
> full_duplex so change the function signature to take an unsigned flags
> argument. Also alter the functions that call save_term() so that they
> can pass flags down to it.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  compat/terminal.c | 41 +++++++++++++++++++++--------------------
>  compat/terminal.h |  5 ++++-
>  2 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/compat/terminal.c b/compat/terminal.c
> index d882dfa06e..bad8e04cd8 100644
> --- a/compat/terminal.c
> +++ b/compat/terminal.c
> @@ -34,7 +34,7 @@ void restore_term(void)
>  	sigchain_pop_common();
>  }
>  
> -int save_term(int full_duplex)
> +int save_term(unsigned flags)

Doing e.g.  ...

>  void restore_term(void)
> diff --git a/compat/terminal.h b/compat/terminal.h
> index 0fb9fa147c..f24b91390d 100644
> --- a/compat/terminal.h
> +++ b/compat/terminal.h
> @@ -1,14 +1,17 @@
>  #ifndef COMPAT_TERMINAL_H
>  #define COMPAT_TERMINAL_H
>  
> +/* Save input and output settings */
> +#define SAVE_TERM_DUPLEX (1u << 0)
	
	enum save_terminal_flags {
		SAVE_TERMINAL_FLAGS = 1 << 0,
	};
	
Here would be better IMO. See 3f9ab7ccdea (parse-options.[ch]:
consistently use "enum parse_opt_flags", 2021-10-08) for how it makes
debugging better.
Phillip Wood March 7, 2022, 10:45 a.m. UTC | #3
On 05/03/2022 14:02, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Mar 04 2022, Phillip Wood wrote:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> The next commit will add another flag in addition to the existing
>> full_duplex so change the function signature to take an unsigned flags
>> argument. Also alter the functions that call save_term() so that they
>> can pass flags down to it.
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   compat/terminal.c | 41 +++++++++++++++++++++--------------------
>>   compat/terminal.h |  5 ++++-
>>   2 files changed, 25 insertions(+), 21 deletions(-)
>>
>> diff --git a/compat/terminal.c b/compat/terminal.c
>> index d882dfa06e..bad8e04cd8 100644
>> --- a/compat/terminal.c
>> +++ b/compat/terminal.c
>> @@ -34,7 +34,7 @@ void restore_term(void)
>>   	sigchain_pop_common();
>>   }
>>   
>> -int save_term(int full_duplex)
>> +int save_term(unsigned flags)
> 
> Doing e.g.  ...
> 
>>   void restore_term(void)
>> diff --git a/compat/terminal.h b/compat/terminal.h
>> index 0fb9fa147c..f24b91390d 100644
>> --- a/compat/terminal.h
>> +++ b/compat/terminal.h
>> @@ -1,14 +1,17 @@
>>   #ifndef COMPAT_TERMINAL_H
>>   #define COMPAT_TERMINAL_H
>>   
>> +/* Save input and output settings */
>> +#define SAVE_TERM_DUPLEX (1u << 0)
> 	
> 	enum save_terminal_flags {
> 		SAVE_TERMINAL_FLAGS = 1 << 0,
> 	};
> 	
> Here would be better IMO. See 3f9ab7ccdea (parse-options.[ch]:
> consistently use "enum parse_opt_flags", 2021-10-08) for how it makes
> debugging better.

I'd remembered Junio objecting to enums for bit flags in the discussion 
of that patch but looking at the whole thread it seems like the debugger 
support lead him to change his mind. I'll update.

Best Wishes

Phillip
Phillip Wood March 7, 2022, 11:11 a.m. UTC | #4
Hi Ramsay

On 04/03/2022 20:40, Ramsay Jones wrote:
> Hi Phillip,
> 
> I have not studied/applied your patches, they are just floating
> past my inbox, so please ignore me if I have misunderstood ...
> 
> On 04/03/2022 13:11, Phillip Wood wrote:
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> The next commit will add another flag in addition to the existing
>> full_duplex so change the function signature to take an unsigned flags
>> argument. Also alter the functions that call save_term() so that they
>> can pass flags down to it.
>>
>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>> ---
>>   compat/terminal.c | 41 +++++++++++++++++++++--------------------
>>   compat/terminal.h |  5 ++++-
>>   2 files changed, 25 insertions(+), 21 deletions(-)
>>
>> diff --git a/compat/terminal.c b/compat/terminal.c
>> index d882dfa06e..bad8e04cd8 100644
>> --- a/compat/terminal.c
>> +++ b/compat/terminal.c
>> @@ -34,7 +34,7 @@ void restore_term(void)
>>   	sigchain_pop_common();
>>   }
>>   
>> -int save_term(int full_duplex)
>> +int save_term(unsigned flags)
>>   {
>>   	if (term_fd < 0)
>>   		term_fd = open("/dev/tty", O_RDWR);
>> @@ -47,11 +47,11 @@ int save_term(int full_duplex)
>>   	return 0;
>>   }
>>   
>> -static int disable_bits(tcflag_t bits)
>> +static int disable_bits(unsigned flags, tcflag_t bits)
> 
> .. you add the 'flags' as the new first parameter ...
> 
>>   {
>>   	struct termios t;
>>   
>> -	if (save_term(0) < 0)
>> +	if (save_term(flags) < 0)
>>   		goto error;
>>   
>>   	t = old_term;
>> @@ -71,14 +71,14 @@ static int disable_bits(tcflag_t bits)
>>   	return -1;
>>   }
>>   
>> -static int disable_echo(void)
>> +static int disable_echo(unsigned flags)
>>   {
>> -	return disable_bits(ECHO);
>> +	return disable_bits(flags, ECHO);
> 
> .. and pass it as first parameter, good, and ...
> 
>>   }
>>   
>> -static int enable_non_canonical(void)
>> +static int enable_non_canonical(unsigned flags)
>>   {
>> -	return disable_bits(ICANON | ECHO);
>> +	return disable_bits(flags, ICANON | ECHO);
> 
> .. here as well, good, and ...
> 
>>   }
>>   
>>   #elif defined(GIT_WINDOWS_NATIVE)
>> @@ -126,15 +126,15 @@ void restore_term(void)
>>   	hconin = hconout = INVALID_HANDLE_VALUE;
>>   }
>>   
>> -int save_term(int full_duplex)
>> +int save_term(unsigned flags)
>>   {
>>   	hconin = CreateFileA("CONIN$", GENERIC_READ | GENERIC_WRITE,
>>   	    FILE_SHARE_READ, NULL, OPEN_EXISTING,
>>   	    FILE_ATTRIBUTE_NORMAL, NULL);
>>   	if (hconin == INVALID_HANDLE_VALUE)
>>   		return -1;
>>   
>> -	if (full_duplex) {
>> +	if (flags & SAVE_TERM_DUPLEX) {
>>   		hconout = CreateFileA("CONOUT$", GENERIC_READ | GENERIC_WRITE,
>>   			FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
>>   			FILE_ATTRIBUTE_NORMAL, NULL);
>> @@ -154,7 +154,7 @@ int save_term(int full_duplex)
>>   	return -1;
>>   }
>>   
>> -static int disable_bits(DWORD bits)
>> +static int disable_bits(unsigned flags, DWORD bits)
> 
> .. Huh? Ah, the DWORD suggests this is in an #ifdef'd windows
> part of the file, OK. ...
> 
>>   {
>>   	if (use_stty) {
>>   		struct child_process cp = CHILD_PROCESS_INIT;
>> @@ -191,7 +191,7 @@ static int disable_bits(DWORD bits)
>>   		use_stty = 0;
>>   	}
>>   
>> -	if (save_term(0) < 0)
>> +	if (save_term(flags) < 0)
>>   		return -1;
>>   
>>   	if (!SetConsoleMode(hconin, cmode_in & ~bits)) {
>> @@ -204,14 +204,15 @@ static int disable_bits(DWORD bits)
>>   	return 0;
>>   }
>>   
>> -static int disable_echo(void)
>> +static int disable_echo(unsigned flags)
>>   {
>> -	return disable_bits(ENABLE_ECHO_INPUT);
>> +	return disable_bits(ENABLE_ECHO_INPUT, flags);
> 
> .. but here, you pass the flags as the second parameter. ;-)

Oh dear that's embarrassing, thanks for your careful review.

Are patches 3 & 4 OK for non-stop platforms?

Best Wishes

Phillip

>>   }
>>   
>> -static int enable_non_canonical(void)
>> +static int enable_non_canonical(unsigned flags)
>>   {
>> -	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);
>> +	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT,
>> +			    flags);
> 
> .. ditto here.
> 
> ATB,
> Ramsay Jones
> 
> 
>>   }
>>   
>>   /*
>> @@ -267,7 +268,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
>>   		return NULL;
>>   	}
>>   
>> -	if (!echo && disable_echo()) {
>> +	if (!echo && disable_echo(0)) {
>>   		fclose(input_fh);
>>   		fclose(output_fh);
>>   		return NULL;
>> @@ -361,7 +362,7 @@ int read_key_without_echo(struct strbuf *buf)
>>   	static int warning_displayed;
>>   	int ch;
>>   
>> -	if (warning_displayed || enable_non_canonical() < 0) {
>> +	if (warning_displayed || enable_non_canonical(0) < 0) {
>>   		if (!warning_displayed) {
>>   			warning("reading single keystrokes not supported on "
>>   				"this platform; reading line instead");
>> @@ -413,10 +414,10 @@ int read_key_without_echo(struct strbuf *buf)
>>   
>>   #else
>>   
>> -int save_term(int full_duplex)
>> +int save_term(unsigned flags)
>>   {
>> -	/* full_duplex == 1, but no support available */
>> -	return -full_duplex;
>> +	/* no duplex support available */
>> +	return -!!(flags & SAVE_TERM_DUPLEX);
>>   }
>>   
>>   void restore_term(void)
>> diff --git a/compat/terminal.h b/compat/terminal.h
>> index 0fb9fa147c..f24b91390d 100644
>> --- a/compat/terminal.h
>> +++ b/compat/terminal.h
>> @@ -1,14 +1,17 @@
>>   #ifndef COMPAT_TERMINAL_H
>>   #define COMPAT_TERMINAL_H
>>   
>> +/* Save input and output settings */
>> +#define SAVE_TERM_DUPLEX (1u << 0)
>> +
>>   /*
>>    * Save the terminal attributes so they can be restored later by a
>>    * call to restore_term(). Note that every successful call to
>>    * save_term() must be matched by a call to restore_term() even if the
>>    * attributes have not been changed. Returns 0 on success, -1 on
>>    * failure.
>>    */
>> -int save_term(int full_duplex);
>> +int save_term(unsigned flags);
>>   /* Restore the terminal attributes that were saved with save_term() */
>>   void restore_term(void);
>>
Ævar Arnfjörð Bjarmason March 7, 2022, 12:06 p.m. UTC | #5
On Mon, Mar 07 2022, Phillip Wood wrote:

> On 05/03/2022 14:02, Ævar Arnfjörð Bjarmason wrote:
>> On Fri, Mar 04 2022, Phillip Wood wrote:
>> 
>>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>>
>>> The next commit will add another flag in addition to the existing
>>> full_duplex so change the function signature to take an unsigned flags
>>> argument. Also alter the functions that call save_term() so that they
>>> can pass flags down to it.
>>>
>>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
>>> ---
>>>   compat/terminal.c | 41 +++++++++++++++++++++--------------------
>>>   compat/terminal.h |  5 ++++-
>>>   2 files changed, 25 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/compat/terminal.c b/compat/terminal.c
>>> index d882dfa06e..bad8e04cd8 100644
>>> --- a/compat/terminal.c
>>> +++ b/compat/terminal.c
>>> @@ -34,7 +34,7 @@ void restore_term(void)
>>>   	sigchain_pop_common();
>>>   }
>>>   -int save_term(int full_duplex)
>>> +int save_term(unsigned flags)
>> Doing e.g.  ...
>> 
>>>   void restore_term(void)
>>> diff --git a/compat/terminal.h b/compat/terminal.h
>>> index 0fb9fa147c..f24b91390d 100644
>>> --- a/compat/terminal.h
>>> +++ b/compat/terminal.h
>>> @@ -1,14 +1,17 @@
>>>   #ifndef COMPAT_TERMINAL_H
>>>   #define COMPAT_TERMINAL_H
>>>   +/* Save input and output settings */
>>> +#define SAVE_TERM_DUPLEX (1u << 0)
>> 	
>> 	enum save_terminal_flags {
>> 		SAVE_TERMINAL_FLAGS = 1 << 0,
>> 	};
>> 	
>> Here would be better IMO. See 3f9ab7ccdea (parse-options.[ch]:
>> consistently use "enum parse_opt_flags", 2021-10-08) for how it makes
>> debugging better.
>
> I'd remembered Junio objecting to enums for bit flags in the
> discussion of that patch but looking at the whole thread it seems like
> the debugger support lead him to change his mind. I'll update.

Yeah, aside from that I think part of that was whether it was worth it
to refactor it for existing code, but since this is new code & we tend
to use that pattern liberally (which pre-dates any recent changes I did
by quite a bit...), ....
Ramsay Jones March 7, 2022, 8:21 p.m. UTC | #6
Hi Phillip,

On 07/03/2022 11:11, Phillip Wood wrote:
> Hi Ramsay
[snip]
>> .. but here, you pass the flags as the second parameter. ;-)
> 
> Oh dear that's embarrassing, thanks for your careful review.
> 
> Are patches 3 & 4 OK for non-stop platforms?

Err... I didn't notice any problems with patches 3 & 4, but, as
far as the non-stop platform is concerned, I wouldn't have a clue! ;-)

(Perhaps you were thinking of Randall?)

ATB,
Ramsay Jones
Phillip Wood March 8, 2022, 10:41 a.m. UTC | #7
Hi Ramsay

On 07/03/2022 20:21, Ramsay Jones wrote:
> Hi Phillip,
> 
> On 07/03/2022 11:11, Phillip Wood wrote:
>> Hi Ramsay
> [snip]
>>> .. but here, you pass the flags as the second parameter. ;-)
>>
>> Oh dear that's embarrassing, thanks for your careful review.
>>
>> Are patches 3 & 4 OK for non-stop platforms?
> 
> Err... I didn't notice any problems with patches 3 & 4, but, as
> far as the non-stop platform is concerned, I wouldn't have a clue! ;-)
> 
> (Perhaps you were thinking of Randall?)

Sorry, yes I had confused you with Randall

Apologies

Phillip
diff mbox series

Patch

diff --git a/compat/terminal.c b/compat/terminal.c
index d882dfa06e..bad8e04cd8 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -34,7 +34,7 @@  void restore_term(void)
 	sigchain_pop_common();
 }
 
-int save_term(int full_duplex)
+int save_term(unsigned flags)
 {
 	if (term_fd < 0)
 		term_fd = open("/dev/tty", O_RDWR);
@@ -47,11 +47,11 @@  int save_term(int full_duplex)
 	return 0;
 }
 
-static int disable_bits(tcflag_t bits)
+static int disable_bits(unsigned flags, tcflag_t bits)
 {
 	struct termios t;
 
-	if (save_term(0) < 0)
+	if (save_term(flags) < 0)
 		goto error;
 
 	t = old_term;
@@ -71,14 +71,14 @@  static int disable_bits(tcflag_t bits)
 	return -1;
 }
 
-static int disable_echo(void)
+static int disable_echo(unsigned flags)
 {
-	return disable_bits(ECHO);
+	return disable_bits(flags, ECHO);
 }
 
-static int enable_non_canonical(void)
+static int enable_non_canonical(unsigned flags)
 {
-	return disable_bits(ICANON | ECHO);
+	return disable_bits(flags, ICANON | ECHO);
 }
 
 #elif defined(GIT_WINDOWS_NATIVE)
@@ -126,15 +126,15 @@  void restore_term(void)
 	hconin = hconout = INVALID_HANDLE_VALUE;
 }
 
-int save_term(int full_duplex)
+int save_term(unsigned flags)
 {
 	hconin = CreateFileA("CONIN$", GENERIC_READ | GENERIC_WRITE,
 	    FILE_SHARE_READ, NULL, OPEN_EXISTING,
 	    FILE_ATTRIBUTE_NORMAL, NULL);
 	if (hconin == INVALID_HANDLE_VALUE)
 		return -1;
 
-	if (full_duplex) {
+	if (flags & SAVE_TERM_DUPLEX) {
 		hconout = CreateFileA("CONOUT$", GENERIC_READ | GENERIC_WRITE,
 			FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
 			FILE_ATTRIBUTE_NORMAL, NULL);
@@ -154,7 +154,7 @@  int save_term(int full_duplex)
 	return -1;
 }
 
-static int disable_bits(DWORD bits)
+static int disable_bits(unsigned flags, DWORD bits)
 {
 	if (use_stty) {
 		struct child_process cp = CHILD_PROCESS_INIT;
@@ -191,7 +191,7 @@  static int disable_bits(DWORD bits)
 		use_stty = 0;
 	}
 
-	if (save_term(0) < 0)
+	if (save_term(flags) < 0)
 		return -1;
 
 	if (!SetConsoleMode(hconin, cmode_in & ~bits)) {
@@ -204,14 +204,15 @@  static int disable_bits(DWORD bits)
 	return 0;
 }
 
-static int disable_echo(void)
+static int disable_echo(unsigned flags)
 {
-	return disable_bits(ENABLE_ECHO_INPUT);
+	return disable_bits(ENABLE_ECHO_INPUT, flags);
 }
 
-static int enable_non_canonical(void)
+static int enable_non_canonical(unsigned flags)
 {
-	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT);
+	return disable_bits(ENABLE_ECHO_INPUT | ENABLE_LINE_INPUT | ENABLE_PROCESSED_INPUT,
+			    flags);
 }
 
 /*
@@ -267,7 +268,7 @@  char *git_terminal_prompt(const char *prompt, int echo)
 		return NULL;
 	}
 
-	if (!echo && disable_echo()) {
+	if (!echo && disable_echo(0)) {
 		fclose(input_fh);
 		fclose(output_fh);
 		return NULL;
@@ -361,7 +362,7 @@  int read_key_without_echo(struct strbuf *buf)
 	static int warning_displayed;
 	int ch;
 
-	if (warning_displayed || enable_non_canonical() < 0) {
+	if (warning_displayed || enable_non_canonical(0) < 0) {
 		if (!warning_displayed) {
 			warning("reading single keystrokes not supported on "
 				"this platform; reading line instead");
@@ -413,10 +414,10 @@  int read_key_without_echo(struct strbuf *buf)
 
 #else
 
-int save_term(int full_duplex)
+int save_term(unsigned flags)
 {
-	/* full_duplex == 1, but no support available */
-	return -full_duplex;
+	/* no duplex support available */
+	return -!!(flags & SAVE_TERM_DUPLEX);
 }
 
 void restore_term(void)
diff --git a/compat/terminal.h b/compat/terminal.h
index 0fb9fa147c..f24b91390d 100644
--- a/compat/terminal.h
+++ b/compat/terminal.h
@@ -1,14 +1,17 @@ 
 #ifndef COMPAT_TERMINAL_H
 #define COMPAT_TERMINAL_H
 
+/* Save input and output settings */
+#define SAVE_TERM_DUPLEX (1u << 0)
+
 /*
  * Save the terminal attributes so they can be restored later by a
  * call to restore_term(). Note that every successful call to
  * save_term() must be matched by a call to restore_term() even if the
  * attributes have not been changed. Returns 0 on success, -1 on
  * failure.
  */
-int save_term(int full_duplex);
+int save_term(unsigned flags);
 /* Restore the terminal attributes that were saved with save_term() */
 void restore_term(void);