From: Ben Keene Date: Mon, 10 Feb 2020 15:49:21 GMT Subject: Re: [PATCH v3 1/5] git-p4: rewrite prompt to be Windows compatible Message-ID: <2e0d4859-a2d7-9882-4781-7ae67ec608f5@gmail.com> In-Reply-To: On 2/6/2020 2:28 PM, Junio C Hamano wrote: > "Ben Keene via GitGitGadget" writes: > >> From: Ben Keene >> >> The existing function prompt(prompt_text) does not work correctly when >> run on Windows 10 bash terminal when launched from the sourcetree >> GUI application. The stdout is not flushed properly so the prompt text >> is not displayed to the user until the next flush of stdout, which is >> quite confusing. > Is that the bug in raw_input(prompt_text) used in the source, or is > that the bug in your environment (whatever "the sourcetree GUI > application" is)? I cannot quite tell if this is butchering code > that is perfectly working well for other people just to cope with a > broken invoker that is what really needs fixing, or if it is working > around a bug in raw_input(). If the former, the change is not what > we want, and if the latter, the change should go to Python upstream, > so either way, I am not sure if we want this patch without further > information. Sourcetree is a graphical UI for GIT maintained by Atlassian. It invokes the git-bash.exe in the git for windows installation as the shell interface. In this context, the raw_input() function failed to work for me.  I do not know if I am unique in using git-p4.py on a windows platform, but from what I could find online, raw_input() does not flush stdout. (See https://bugs.python.org/issue526382) From what I saw in response to this problem, everyone suggested reading from stdin instead of using raw_input. Though I can understand the hesitation for this sort of change. > > Anybody on a similar platform have opinion on this? I am OK as long > as this change does *not* break the program in an environment that > it is working fine, but that is not even clear. > >> Change this method by: >> * Adding flush to stderr, stdout, and stdin >> * Use readline from sys.stdin instead of raw_input. >> >> The existing strip().lower() are retained. >> >> Signed-off-by: Ben Keene >> --- >> git-p4.py | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/git-p4.py b/git-p4.py >> index 40d9e7c594..7d8a5ee788 100755 >> --- a/git-p4.py >> +++ b/git-p4.py >> @@ -175,7 +175,11 @@ def prompt(prompt_text): >> """ >> choices = set(m.group(1) for m in re.finditer(r"\[(.)\]", prompt_text)) >> while True: >> - response = raw_input(prompt_text).strip().lower() >> + sys.stderr.flush() >> + sys.stdout.write(prompt_text) >> + sys.stdout.flush() > raw_input() is getting replace with input() in another series to > bring us to Python3 compatible world, but because you are getting > rid of its use, as long as the resulting code works with both > Python2 and Python3, we are happy ;-) > >> + sys.stdin.flush() > What does it even mean to flush the input stream here? At least in > C, it is meaningless and causes an undefined behaviour. The intent, was to clear out the input buffer before reading the input, however, in a non-interactive mode, this would be undesirable. I will remove the stdin.flush(). > >> + response=sys.stdin.readline().strip().lower() >> if not response: >> continue >> response = response[0] > Thanks.