Patchwork [2,of,2] py3: flush ui after each message in interactive patch filtering

login
register
mail settings
Submitter Denis Laxalde
Date Oct. 9, 2019, 1:29 p.m.
Message ID <0cdeb24af4076eae433d.1570627764@steppe.local>
Download mbox | patch
Permalink /patch/42135/
State Accepted
Headers show

Comments

Denis Laxalde - Oct. 9, 2019, 1:29 p.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1570627454 -7200
#      Wed Oct 09 15:24:14 2019 +0200
# Node ID 0cdeb24af4076eae433de3f63fda36da4e6fc0d0
# Parent  99f06850be3ecc521ba461684686865c1191c5a0
py3: flush ui after each message in interactive patch filtering

Otherwise, actions from ui.write() are buffered and displayed at end of
interactive session.
Yuya Nishihara - Oct. 9, 2019, 2:31 p.m.
On Wed, 09 Oct 2019 15:29:24 +0200, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1570627454 -7200
> #      Wed Oct 09 15:24:14 2019 +0200
> # Node ID 0cdeb24af4076eae433de3f63fda36da4e6fc0d0
> # Parent  99f06850be3ecc521ba461684686865c1191c5a0
> py3: flush ui after each message in interactive patch filtering

Queued, thanks.

> Otherwise, actions from ui.write() are buffered and displayed at end of
> interactive session.

I suspect that "sys.stdout.buffer" isn't line-buffered on Python 3 even
though "sys.stdout" is documented as such. If that's true, we'll need another
hack to make our stdout do the right thing.

https://docs.python.org/3/library/sys.html#sys.stdout
Pulkit Goyal - Oct. 9, 2019, 2:42 p.m.
On Wed, Oct 9, 2019 at 4:42 PM Denis Laxalde <denis@laxalde.org> wrote:
>
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1570627454 -7200
> #      Wed Oct 09 15:24:14 2019 +0200
> # Node ID 0cdeb24af4076eae433de3f63fda36da4e6fc0d0
> # Parent  99f06850be3ecc521ba461684686865c1191c5a0
> py3: flush ui after each message in interactive patch filtering
>
> Otherwise, actions from ui.write() are buffered and displayed at end of
> interactive session.

According to nbjoerg on IRC, it's better to flush before asking for
input instead of flushing after every write. Queued only 1.
Denis Laxalde - Oct. 10, 2019, 1:19 p.m.
Yuya Nishihara a écrit :
> I suspect that "sys.stdout.buffer" isn't line-buffered on Python 3 even
> though "sys.stdout" is documented as such. If that's true, we'll need another
> hack to make our stdout do the right thing.
> 
> https://docs.python.org/3/library/sys.html#sys.stdout

As far as I can tell, "sys.stdout.buffer" is line-buffered on python3. It
seems to me that the problem is that we're not actually using this but
procutil.stdout as ui._fout; and procutil happens to replace stdout
(initially "sys.stdout.buffer") by "os.fdopen(stdout.fileno(), r'wb',
1)". But apparently, the buffering=1 argument is not usable for binary
mode, according to:

  https://docs.python.org/3/library/functions.html#open
  https://docs.python.org/3/library/os.html#os.fdopen

I would then suggest to not replace stdout in procutil when using
python3 (and keep sys.stdout.buffer). Would that be correct?

(There are more such issues, e.g. in "hg email", so it's indeed worth
a correct fix.)
Yuya Nishihara - Oct. 10, 2019, 1:41 p.m.
On Thu, 10 Oct 2019 15:19:37 +0200, Denis Laxalde wrote:
> Yuya Nishihara a écrit :
> > I suspect that "sys.stdout.buffer" isn't line-buffered on Python 3 even
> > though "sys.stdout" is documented as such. If that's true, we'll need another
> > hack to make our stdout do the right thing.
> > 
> > https://docs.python.org/3/library/sys.html#sys.stdout
> 
> As far as I can tell, "sys.stdout.buffer" is line-buffered on python3. It
> seems to me that the problem is that we're not actually using this but
> procutil.stdout as ui._fout; and procutil happens to replace stdout
> (initially "sys.stdout.buffer") by "os.fdopen(stdout.fileno(), r'wb',
> 1)". But apparently, the buffering=1 argument is not usable for binary
> mode, according to:

Interesting. So bytes I/O doesn't support line-buffering?

>   https://docs.python.org/3/library/functions.html#open
>   https://docs.python.org/3/library/os.html#os.fdopen
> 
> I would then suggest to not replace stdout in procutil when using
> python3 (and keep sys.stdout.buffer). Would that be correct?

Maybe. The hack in procutil.py exists because there's no standard way
to change the buffering mode of the underlying FILE object. If we can
explicitly make sys.stdout.buffer line-buffered, we won't have to reopen
the stdout.

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
index 374d5d6..e27d0ca 100644
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1188,9 +1188,11 @@  def filterpatch(ui, headers, match, oper
             # chars is a good target) because of issue6158.
             r = ui.promptchoice(b"%s\n(enter ? for help) %s" % (query, resps))
             ui.write(b"\n")
+            ui.flush()
             if r == 8:  # ?
                 for c, t in ui.extractchoices(resps)[1]:
                     ui.write(b'%s - %s\n' % (c, encoding.lower(t)))
+                    ui.flush()
                 continue
             elif r == 0:  # yes
                 ret = True
@@ -1200,10 +1202,12 @@  def filterpatch(ui, headers, match, oper
                 if chunk is None:
                     ui.write(_(b'cannot edit patch for whole file'))
                     ui.write(b"\n")
+                    ui.flush()
                     continue
                 if chunk.header.binary():
                     ui.write(_(b'cannot edit patch for binary file'))
                     ui.write(b"\n")
+                    ui.flush()
                     continue
                 # Patch comment based on the Git one (based on comment at end of
                 # https://mercurial-scm.org/wiki/RecordExtension)
@@ -1304,6 +1308,7 @@  the hunk is left unchanged.
         for i, chunk in enumerate(h.hunks):
             if skipfile is None and skipall is None:
                 chunk.pretty(ui)
+                ui.flush()
             if total == 1:
                 msg = messages[b'single'][operation] % chunk.filename()
             else: