Patchwork [2,of,2,V2] dispatch: flush ui before returning from _runcatch

login
register
mail settings
Submitter Jun Wu
Date March 14, 2016, 12:20 p.m.
Message ID <40b571e817902d930d1d.1457958036@x1c>
Download mbox | patch
Permalink /patch/13860/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - March 14, 2016, 12:20 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1457953594 0
#      Mon Mar 14 11:06:34 2016 +0000
# Node ID 40b571e817902d930d1d828769d2d29bbba88de1
# Parent  37c1410031153c71a46a467e6c94af16a7266d02
dispatch: flush ui before returning from _runcatch

A chg client may exit after received the result from runcommand. It is
necessary to do a flush to make sure the warning message is printed out
and the process waiting for the chg client will actually see the output.

This helps chg to pass test-alias.t.
Yuya Nishihara - March 14, 2016, 3:09 p.m.
On Mon, 14 Mar 2016 12:20:36 +0000, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1457953594 0
> #      Mon Mar 14 11:06:34 2016 +0000
> # Node ID 40b571e817902d930d1d828769d2d29bbba88de1
> # Parent  37c1410031153c71a46a467e6c94af16a7266d02
> dispatch: flush ui before returning from _runcatch
> 
> A chg client may exit after received the result from runcommand. It is
> necessary to do a flush to make sure the warning message is printed out
> and the process waiting for the chg client will actually see the output.
> 
> This helps chg to pass test-alias.t.
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -384,6 +384,7 @@
>          ui.warn(warning)
>          raise
>  
> +    ui.flush()
>      return -1

I guess this wouldn't catch the all cases. I'll recheck it tomorrow.
Jun Wu - March 14, 2016, 4:14 p.m.
On 03/14/2016 03:09 PM, Yuya Nishihara wrote:
> I guess this wouldn't catch the all cases. I'll recheck it tomorrow.

No, it wouldn't. Namely ParseError has "return -1" (which I think can be
removed) and re-raise.

I didn't put it in "finally" because the inner try block has a "finally" 
already. I think it may be better to do "ui.flush()" only once.

Now I prefer adding a "finally" here.
Yuya Nishihara - March 15, 2016, 2:58 p.m.
On Mon, 14 Mar 2016 16:14:43 +0000, Jun Wu wrote:
> On 03/14/2016 03:09 PM, Yuya Nishihara wrote:
> > I guess this wouldn't catch the all cases. I'll recheck it tomorrow.
> 
> No, it wouldn't. Namely ParseError has "return -1" (which I think can be
> removed) and re-raise.
> 
> I didn't put it in "finally" because the inner try block has a "finally"
> already. I think it may be better to do "ui.flush()" only once.

I guess that ui.flush() would be necessary before entering to the debugger
session.

> Now I prefer adding a "finally" here.

Can you send new version? I think ui.flush() can be done at the finally block
of dispatch().

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -384,6 +384,7 @@ 
         ui.warn(warning)
         raise
 
+    ui.flush()
     return -1
 
 def aliasargs(fn, givenargs):