Patchwork [6,of,6] blackbox: trap warnings

login
register
mail settings
Submitter timeless@mozdev.org
Date March 1, 2016, 11:51 a.m.
Message ID <ac01762261adcbf4d996.1456833108@waste.org>
Download mbox | patch
Permalink /patch/13501/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

timeless@mozdev.org - March 1, 2016, 11:51 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1455219585 0
#      Thu Feb 11 19:39:45 2016 +0000
# Node ID ac01762261adcbf4d996f1a39da0b28c91e2ad34
# Parent  0191718c65ff72d16c42839e4be126d9b565d787
blackbox: trap warnings
Yuya Nishihara - March 3, 2016, 1:26 p.m.
On Tue, 01 Mar 2016 05:51:48 -0600, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1455219585 0
> #      Thu Feb 11 19:39:45 2016 +0000
> # Node ID ac01762261adcbf4d996f1a39da0b28c91e2ad34
> # Parent  0191718c65ff72d16c42839e4be126d9b565d787
> blackbox: trap warnings
> 
> diff --git a/hgext/blackbox.py b/hgext/blackbox.py
> --- a/hgext/blackbox.py
> +++ b/hgext/blackbox.py
> @@ -189,6 +189,11 @@
>                  if not lastui or ui._bbrepo:
>                      lastui = ui
>  
> +        def warn(self, *msg, **opts):
> +            super(blackboxui, self).warn(*msg, **opts)
> +            output = ''.join(msg)
> +            self.log("blackboxwarn", "%s\n", output)

Ugh, do we really want to capture all warnings?
timeless - March 3, 2016, 1:57 p.m.
I can put this behind a flag...
On Mar 3, 2016 8:28 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:

> On Tue, 01 Mar 2016 05:51:48 -0600, timeless wrote:
> > # HG changeset patch
> > # User timeless <timeless@mozdev.org>
> > # Date 1455219585 0
> > #      Thu Feb 11 19:39:45 2016 +0000
> > # Node ID ac01762261adcbf4d996f1a39da0b28c91e2ad34
> > # Parent  0191718c65ff72d16c42839e4be126d9b565d787
> > blackbox: trap warnings
> >
> > diff --git a/hgext/blackbox.py b/hgext/blackbox.py
> > --- a/hgext/blackbox.py
> > +++ b/hgext/blackbox.py
> > @@ -189,6 +189,11 @@
> >                  if not lastui or ui._bbrepo:
> >                      lastui = ui
> >
> > +        def warn(self, *msg, **opts):
> > +            super(blackboxui, self).warn(*msg, **opts)
> > +            output = ''.join(msg)
> > +            self.log("blackboxwarn", "%s\n", output)
>
> Ugh, do we really want to capture all warnings?
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - March 3, 2016, 4:48 p.m.
On Thu, 3 Mar 2016 08:57:17 -0500, timeless wrote:
> I can put this behind a flag...

A flag won't be necessary because the log is filtered by blackbox.track.
My concern is about the design. I'm not the right person who can tell
the design of the blackbox extension.

> On Mar 3, 2016 8:28 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:
> > On Tue, 01 Mar 2016 05:51:48 -0600, timeless wrote:
> > > # HG changeset patch
> > > # User timeless <timeless@mozdev.org>
> > > # Date 1455219585 0
> > > #      Thu Feb 11 19:39:45 2016 +0000
> > > # Node ID ac01762261adcbf4d996f1a39da0b28c91e2ad34
> > > # Parent  0191718c65ff72d16c42839e4be126d9b565d787
> > > blackbox: trap warnings
> > >
> > > diff --git a/hgext/blackbox.py b/hgext/blackbox.py
> > > --- a/hgext/blackbox.py
> > > +++ b/hgext/blackbox.py
> > > @@ -189,6 +189,11 @@
> > >                  if not lastui or ui._bbrepo:
> > >                      lastui = ui
> > >
> > > +        def warn(self, *msg, **opts):
> > > +            super(blackboxui, self).warn(*msg, **opts)
> > > +            output = ''.join(msg)
> > > +            self.log("blackboxwarn", "%s\n", output)
> >
> > Ugh, do we really want to capture all warnings?
Pierre-Yves David - March 9, 2016, 4:56 p.m.
On 03/03/2016 04:48 PM, Yuya Nishihara wrote:
> On Thu, 3 Mar 2016 08:57:17 -0500, timeless wrote:
>> I can put this behind a flag...
>
> A flag won't be necessary because the log is filtered by blackbox.track.
> My concern is about the design. I'm not the right person who can tell
> the design of the blackbox extension.

So what's the plan here?

>
>> On Mar 3, 2016 8:28 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:
>>> On Tue, 01 Mar 2016 05:51:48 -0600, timeless wrote:
>>>> # HG changeset patch
>>>> # User timeless <timeless@mozdev.org>
>>>> # Date 1455219585 0
>>>> #      Thu Feb 11 19:39:45 2016 +0000
>>>> # Node ID ac01762261adcbf4d996f1a39da0b28c91e2ad34
>>>> # Parent  0191718c65ff72d16c42839e4be126d9b565d787
>>>> blackbox: trap warnings
>>>>
>>>> diff --git a/hgext/blackbox.py b/hgext/blackbox.py
>>>> --- a/hgext/blackbox.py
>>>> +++ b/hgext/blackbox.py
>>>> @@ -189,6 +189,11 @@
>>>>                   if not lastui or ui._bbrepo:
>>>>                       lastui = ui
>>>>
>>>> +        def warn(self, *msg, **opts):
>>>> +            super(blackboxui, self).warn(*msg, **opts)
>>>> +            output = ''.join(msg)
>>>> +            self.log("blackboxwarn", "%s\n", output)
>>>
>>> Ugh, do we really want to capture all warnings?
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - March 11, 2016, 1:38 p.m.
On Wed, 9 Mar 2016 16:56:59 +0000, Pierre-Yves David wrote:
> On 03/03/2016 04:48 PM, Yuya Nishihara wrote:
> > On Thu, 3 Mar 2016 08:57:17 -0500, timeless wrote:
> >> I can put this behind a flag...
> >
> > A flag won't be necessary because the log is filtered by blackbox.track.
> > My concern is about the design. I'm not the right person who can tell
> > the design of the blackbox extension.
> 
> So what's the plan here?

I hope the patch 4-6 will be reviewed by someone else who knows the internals
of the blackbox. This and the previous patch do bulk capturing of warnings,
which is very different from how ui.log() is currently used.
Pierre-Yves David - March 11, 2016, 1:42 p.m.
On 03/11/2016 01:38 PM, Yuya Nishihara wrote:
> On Wed, 9 Mar 2016 16:56:59 +0000, Pierre-Yves David wrote:
>> On 03/03/2016 04:48 PM, Yuya Nishihara wrote:
>>> On Thu, 3 Mar 2016 08:57:17 -0500, timeless wrote:
>>>> I can put this behind a flag...
>>>
>>> A flag won't be necessary because the log is filtered by blackbox.track.
>>> My concern is about the design. I'm not the right person who can tell
>>> the design of the blackbox extension.
>>
>> So what's the plan here?
>
> I hope the patch 4-6 will be reviewed by someone else who knows the internals
> of the blackbox. This and the previous patch do bulk capturing of warnings,
> which is very different from how ui.log() is currently used.

Timeless, can we get a V2 with these three patch (and initial yuya 
comment fixed) so reboot the review on this ?

Cheers,

Patch

diff --git a/hgext/blackbox.py b/hgext/blackbox.py
--- a/hgext/blackbox.py
+++ b/hgext/blackbox.py
@@ -189,6 +189,11 @@ 
                 if not lastui or ui._bbrepo:
                     lastui = ui
 
+        def warn(self, *msg, **opts):
+            super(blackboxui, self).warn(*msg, **opts)
+            output = ''.join(msg)
+            self.log("blackboxwarn", "%s\n", output)
+
         def setrepo(self, repo):
             self._bbfp = None
             self._bbrepo = repo