Patchwork [2,of,3,v4] diffopts: notice a negated boolean flag in diffopts

login
register
mail settings
Submitter Augie Fackler
Date Sept. 16, 2016, 3:15 p.m.
Message ID <aea18fa52d954e234fdf.1474038902@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/16650/
State Accepted
Headers show

Comments

Augie Fackler - Sept. 16, 2016, 3:15 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1472586907 14400
#      Tue Aug 30 15:55:07 2016 -0400
# Node ID aea18fa52d954e234fdfd1d24d3f37f0cb03dc60
# Parent  cb57a762749f29065635d0480bf5c87c644ba0c5
diffopts: notice a negated boolean flag in diffopts

This means that if you have git-diffs enabled by default (pretty
common) and you hit the rare (but real) case where a git-diff breaks
patch(1) or some other tool, you can easily disable it by just
specifying --no-git on the command line.

I feel a little bad about the isinstance() check, but some values in
diffopts are not booleans and so we need to preserve false iff the
flag is a boolean flag: failing to do this means we end up with empty
string defaults for flags clobbering meaningful values from the [diff]
section in hgrc.
Yuya Nishihara - Sept. 17, 2016, 7:53 a.m.
On Fri, 16 Sep 2016 11:15:02 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1472586907 14400
> #      Tue Aug 30 15:55:07 2016 -0400
> # Node ID aea18fa52d954e234fdfd1d24d3f37f0cb03dc60
> # Parent  cb57a762749f29065635d0480bf5c87c644ba0c5
> diffopts: notice a negated boolean flag in diffopts

Also queued this, thanks.

> I feel a little bad about the isinstance() check, but some values in
> diffopts are not booleans and so we need to preserve false iff the
> flag is a boolean flag: failing to do this means we end up with empty
> string defaults for flags clobbering meaningful values from the [diff]
> section in hgrc.
> 
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -2144,7 +2144,14 @@ def difffeatureopts(ui, opts=None, untru
>      def get(key, name=None, getter=ui.configbool, forceplain=None):
>          if opts:
>              v = opts.get(key)
> -            if v:
> +            # diffopts flags are either None-default (which is passed
> +            # through unchanged, so we can identify unset values), or
> +            # some other falsey default (eg --unified, which defaults
> +            # to an empty string). We only want to override the config
> +            # entries from hgrc with command line values if they
> +            # appear to have been set, which is any truthy value,
> +            # True, or False.
> +            if v or isinstance(v, bool):
>                  return v

Perhaps this could be slightly simpler if we split get() to getbool() and
getstr(), and use v is None, but that isn't a big deal.

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2144,7 +2144,14 @@  def difffeatureopts(ui, opts=None, untru
     def get(key, name=None, getter=ui.configbool, forceplain=None):
         if opts:
             v = opts.get(key)
-            if v:
+            # diffopts flags are either None-default (which is passed
+            # through unchanged, so we can identify unset values), or
+            # some other falsey default (eg --unified, which defaults
+            # to an empty string). We only want to override the config
+            # entries from hgrc with command line values if they
+            # appear to have been set, which is any truthy value,
+            # True, or False.
+            if v or isinstance(v, bool):
                 return v
         if forceplain is not None and ui.plain():
             return forceplain
diff --git a/tests/test-diff-unified.t b/tests/test-diff-unified.t
--- a/tests/test-diff-unified.t
+++ b/tests/test-diff-unified.t
@@ -333,4 +333,20 @@  showfunc diff
   +    return a + b + c + e;
    }
 
+If [diff] git is set to true, but the user says --no-git, we should
+*not* get git diffs
+  $ hg diff --nodates --config diff.git=1 --no-git
+  diff -r f2c7c817fa55 f1
+  --- a/f1
+  +++ b/f1
+  @@ -2,6 +2,6 @@
+       int a = 0;
+       int b = 1;
+       int c = 2;
+  -    int d = 3;
+  -    return a + b + c + d;
+  +    int e = 3;
+  +    return a + b + c + e;
+   }
+
   $ cd ..