Patchwork [5,of,9] patch.diffopts: break get function into if statements

login
register
mail settings
Submitter Siddharth Agarwal
Date Nov. 13, 2014, 8:22 a.m.
Message ID <dc2873c221516113721e.1415866933@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/6701/
State Superseded
Commit ac072c79bd9d4005d793eb94a01d5cb961e7f7bc
Headers show

Comments

Siddharth Agarwal - Nov. 13, 2014, 8:22 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1415864657 28800
#      Wed Nov 12 23:44:17 2014 -0800
# Node ID dc2873c221516113721e604564e7832befbd91ee
# Parent  7448e2dc8b9141202eeafa310b74d3b2e3d1c882
patch.diffopts: break get function into if statements

We're going to add another condition here, and with the current structure that
becomes just too confusing.
Martin von Zweigbergk - Nov. 13, 2014, 8:59 p.m.
On Thu Nov 13 2014 at 12:22:28 AM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1415864657 28800
> #      Wed Nov 12 23:44:17 2014 -0800
> # Node ID dc2873c221516113721e604564e7832befbd91ee
> # Parent  7448e2dc8b9141202eeafa310b74d3b2e3d1c882
> patch.diffopts: break get function into if statements
>
> We're going to add another condition here, and with the current structure
> that
> becomes just too confusing.
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -1559,10 +1559,13 @@
>      pass
>
>  def diffopts(ui, opts=None, untrusted=False, section='diff'):
> -    def get(key, name=None, getter=ui.configbool, forceplain=False):
> -        return ((opts and opts.get(key)) or
> -                (ui.plain() and forceplain) or
> -                getter(section, name or key, None, untrusted=untrusted))
> +    def get(key, name=None, getter=ui.configbool, forceplain=None):
> +        if opts:
> +            v = opts.get(key)
> +            if v:
> +                return v
> +        return getter(section, name or key, None, untrusted=untrusted)
> +
>

 Did "(ui.plain() and forceplain)" get lost by mistake? If not, maybe some
explanation is needed.
Siddharth Agarwal - Nov. 13, 2014, 9:24 p.m.
On 11/13/2014 12:59 PM, Martin von Zweigbergk wrote:
>
>
> On Thu Nov 13 2014 at 12:22:28 AM Siddharth Agarwal <sid0@fb.com 
> <mailto:sid0@fb.com>> wrote:
>
>     # HG changeset patch
>     # User Siddharth Agarwal <sid0@fb.com <mailto:sid0@fb.com>>
>     # Date 1415864657 28800
>     #      Wed Nov 12 23:44:17 2014 -0800
>     # Node ID dc2873c221516113721e604564e7832befbd91ee
>     # Parent  7448e2dc8b9141202eeafa310b74d3b2e3d1c882
>     patch.diffopts: break get function into if statements
>
>     We're going to add another condition here, and with the current
>     structure that
>     becomes just too confusing.
>
>     diff --git a/mercurial/patch.py b/mercurial/patch.py
>     --- a/mercurial/patch.py
>     +++ b/mercurial/patch.py
>     @@ -1559,10 +1559,13 @@
>          pass
>
>      def diffopts(ui, opts=None, untrusted=False, section='diff'):
>     -    def get(key, name=None, getter=ui.configbool, forceplain=False):
>     -        return ((opts and opts.get(key)) or
>     -                (ui.plain() and forceplain) or
>     -                getter(section, name or key, None,
>     untrusted=untrusted))
>     +    def get(key, name=None, getter=ui.configbool, forceplain=None):
>     +        if opts:
>     +            v = opts.get(key)
>     +            if v:
>     +                return v
>     +        return getter(section, name or key, None,
>     untrusted=untrusted)
>     +
>
>
>  Did "(ui.plain() and forceplain)" get lost by mistake? If not, maybe 
> some explanation is needed.

Doh, patches 4 and 5 should have been folded into one. Thanks for 
spotting this. I'll send a V2 with your comments.

- Siddharth

>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1559,10 +1559,13 @@ 
     pass
 
 def diffopts(ui, opts=None, untrusted=False, section='diff'):
-    def get(key, name=None, getter=ui.configbool, forceplain=False):
-        return ((opts and opts.get(key)) or
-                (ui.plain() and forceplain) or
-                getter(section, name or key, None, untrusted=untrusted))
+    def get(key, name=None, getter=ui.configbool, forceplain=None):
+        if opts:
+            v = opts.get(key)
+            if v:
+                return v
+        return getter(section, name or key, None, untrusted=untrusted)
+
     return mdiff.diffopts(
         text=opts and opts.get('text'),
         git=get('git'),