Patchwork [7,of,8,v2] diffopts: notice a negated boolean flag in diffopts

login
register
mail settings
Submitter Augie Fackler
Date Aug. 30, 2016, 8:16 p.m.
Message ID <fd68fb86c29873eb32c4.1472588180@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/16493/
State Changes Requested
Headers show

Comments

Augie Fackler - Aug. 30, 2016, 8:16 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1472586907 14400
#      Tue Aug 30 15:55:07 2016 -0400
# Node ID fd68fb86c29873eb32c4a2bd28f7ac0abe3dc172
# Parent  6127fee6ac8208a62f718d53aca6cb814f43b742
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 --git=0 on the command line.
Yuya Nishihara - Sept. 1, 2016, 3:28 p.m.
On Tue, 30 Aug 2016 16:16:20 -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 fd68fb86c29873eb32c4a2bd28f7ac0abe3dc172
> # Parent  6127fee6ac8208a62f718d53aca6cb814f43b742
> 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 --git=0 on the command line.
> 
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -2142,7 +2142,7 @@ def difffeatureopts(ui, opts=None, untru
>      def get(key, name=None, getter=ui.configbool, forceplain=None):
>          if opts:
>              v = opts.get(key)
> -            if v:
> +            if v or v is False:
>                  return v

Can't we have default=unset to omit the dict element instead of setting it
to None?

  # fancyopts.py
  unset = object()

  # commands.py
  unset = fancyopts.unset
  diffopts = [
      ('g', 'git', unset, _('use git extended diff format')),
  ]

  (none) => {}
  --git => {'git': True}
  --no-git => {'git': False}

I think it will avoid introducing the weirdness of tri-state booleans globally.
Many commands only need False/True. They are free to set default=True and
get False by --no-<opt>.
Pierre-Yves David - Sept. 6, 2016, 4 p.m.
On 08/30/2016 10:16 PM, 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 fd68fb86c29873eb32c4a2bd28f7ac0abe3dc172
> # Parent  6127fee6ac8208a62f718d53aca6cb814f43b742
> 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 --git=0 on the command line.

from the rest of the series, I would expect this '--git=0' to be 
'--no-git'. The test seems to match this expectation of mine.

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2142,7 +2142,7 @@  def difffeatureopts(ui, opts=None, untru
     def get(key, name=None, getter=ui.configbool, forceplain=None):
         if opts:
             v = opts.get(key)
-            if v:
+            if v or v is False:
                 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 ..