Patchwork ui: add prompt argument to write (issue5154) (API)

login
register
mail settings
Submitter timeless@mozdev.org
Date March 25, 2016, 10:06 p.m.
Message ID <bf9ecf0ce88dcd1cfdac.1458943589@waste.org>
Download mbox | patch
Permalink /patch/14075/
State Accepted
Headers show

Comments

timeless@mozdev.org - March 25, 2016, 10:06 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1458942660 0
#      Fri Mar 25 21:51:00 2016 +0000
# Node ID bf9ecf0ce88dcd1cfdac848eb455fd5252d68392
# Parent  345f4fa4cc8912bb722ad3e35d68858487420bc6
ui: add prompt argument to write (issue5154) (API)

When code like filemerge._iprompt calls ui.prompt, it expects
the user to see the output in addition to getting the prompt.

Other code such as histedit may call ui.pushbuffer, but its
goal is not to interfere with prompts, so this commit adds
an optional prompt flag to ui.write and has _readline
include that argument.

ui.promptchoice calls ui.prompt which calls ui._readline.

This commit also updates hgext.color.write.
Pierre-Yves David - March 27, 2016, 6:51 p.m.
On 03/25/2016 03:06 PM, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1458942660 0
> #      Fri Mar 25 21:51:00 2016 +0000
> # Node ID bf9ecf0ce88dcd1cfdac848eb455fd5252d68392
> # Parent  345f4fa4cc8912bb722ad3e35d68858487420bc6
> ui: add prompt argument to write (issue5154) (API)
>
> When code like filemerge._iprompt calls ui.prompt, it expects
> the user to see the output in addition to getting the prompt.
>
> Other code such as histedit may call ui.pushbuffer, but its
> goal is not to interfere with prompts, so this commit adds
> an optional prompt flag to ui.write and has _readline
> include that argument.
>
> ui.promptchoice calls ui.prompt which calls ui._readline.
>
> This commit also updates hgext.color.write.

I've taken this because the bug is pretty bad. But just applying this 
hack will lead to surprise and other bug. Can you follow up with patches:

- adding a developer warning when this happen? This would let the test 
catch such situation in the tests?

- updating histedit to not get into this situation anymore.

Cheers,

Patch

diff --git a/hgext/color.py b/hgext/color.py
--- a/hgext/color.py
+++ b/hgext/color.py
@@ -424,7 +424,7 @@ 
             return super(colorui, self).write(*args, **opts)
 
         label = opts.get('label', '')
-        if self._buffers:
+        if self._buffers and not opts.get('prompt', False):
             if self._bufferapplylabels:
                 self._buffers[-1].extend(self.label(a, label) for a in args)
             else:
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -662,7 +662,7 @@ 
         "cmdname.type" is recommended. For example, status issues
         a label of "status.modified" for modified files.
         '''
-        if self._buffers:
+        if self._buffers and not opts.get('prompt', False):
             self._buffers[-1].extend(a for a in args)
         else:
             self._progclear()
@@ -842,7 +842,7 @@ 
 
         # call write() so output goes through subclassed implementation
         # e.g. color extension on Windows
-        self.write(prompt)
+        self.write(prompt, prompt=True)
 
         # instead of trying to emulate raw_input, swap (self.fin,
         # self.fout) with (sys.stdin, sys.stdout)