Submitter | Augie Fackler |
---|---|
Date | Aug. 18, 2016, 4:13 p.m. |
Message ID | <9ec3aad0bf74ad1c25e8.1471536813@augie-macbookair2.roam.corp.google.com> |
Download | mbox | patch |
Permalink | /patch/16356/ |
State | Accepted |
Headers | show |
Comments
On Thu, Aug 18, 2016 at 9:13 AM, Augie Fackler <raf@durin42.com> wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1471534453 14400 > # Thu Aug 18 11:34:13 2016 -0400 > # Node ID 9ec3aad0bf74ad1c25e87bcd0f536896d1c355ca > # Parent 8ed7fdc716ffc95a4062875eafc2739697122fbc > flags: allow specifying --boolean-flag=(true|false) on the command line > (BC) > Do we want to describe this (in this description) as supporting all of the things parseable by parsebool? Not sure how much it matters to be pedantically correct in the descriptions. Is there documentation that needs to be updated to mention this is now possible? (might be already done in another change in this series, but I didn't see it) > This makes it much easier to enable some anti-foot-shooting features > (like update --check) by default, because now all boolean flags can be > explicitly disabled on the command line without having to use HGPLAIN > or similar. > > diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py > --- a/mercurial/fancyopts.py > +++ b/mercurial/fancyopts.py > @@ -10,7 +10,10 @@ from __future__ import absolute_import > import getopt > > from .i18n import _ > -from . import error > +from . import ( > + error, > + util, > +) > > def gnugetopt(args, options, longoptions): > """Parse options mostly like getopt.gnu_getopt. > @@ -91,6 +94,10 @@ def fancyopts(args, options, state, gnu= > short += ':' > if oname: > oname += '=' > + else: > + prefix = '--' + oname + '=' > + if any(a.startswith(prefix) for a in args): > + oname += '=' > if short: > shortlist += short > if name: > @@ -121,7 +128,10 @@ def fancyopts(args, options, state, gnu= > elif t is type([]): > state[name].append(val) > elif t is type(None) or t is type(False): > - state[name] = True > + if val: > + state[name] = util.parsebool(val) > + else: > + state[name] = True > > # return unparsed args > return args > diff --git a/tests/test-update-branches.t b/tests/test-update-branches.t > --- a/tests/test-update-branches.t > +++ b/tests/test-update-branches.t > @@ -379,3 +379,14 @@ Test experimental revset support > > $ hg log -r '_destupdate()' > 2:bd10386d478c 2 (no-eol) > + > +Test that boolean flags allow --flag=false specification to override > [defaults] > + $ cat >> $HGRCPATH <<EOF > + > [defaults] > + > update = --check > + > EOF > + $ hg co 2 > + abort: uncommitted changes > + [255] > + $ hg co --check=no 2 > + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
> On Aug 18, 2016, at 15:52, Kyle Lippincott <spectral@pewpew.net> wrote: > > > > On Thu, Aug 18, 2016 at 9:13 AM, Augie Fackler <raf@durin42.com <mailto:raf@durin42.com>> wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com <mailto:augie@google.com>> > # Date 1471534453 14400 > # Thu Aug 18 11:34:13 2016 -0400 > # Node ID 9ec3aad0bf74ad1c25e87bcd0f536896d1c355ca > # Parent 8ed7fdc716ffc95a4062875eafc2739697122fbc > flags: allow specifying --boolean-flag=(true|false) on the command line (BC) > > Do we want to describe this (in this description) as supporting all of the things parseable by parsebool? Not sure how much it matters to be pedantically correct in the descriptions. Probably should be in the commit message, yeah. > Is there documentation that needs to be updated to mention this is now possible? (might be already done in another change in this series, but I didn't see it) I'm honestly not sure where to tuck this doc-wise, given that it matters for all boolean flags everywhere. Sigh. I'm very open to ideas... > > > This makes it much easier to enable some anti-foot-shooting features > (like update --check) by default, because now all boolean flags can be > explicitly disabled on the command line without having to use HGPLAIN > or similar. > > diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py > --- a/mercurial/fancyopts.py > +++ b/mercurial/fancyopts.py > @@ -10,7 +10,10 @@ from __future__ import absolute_import > import getopt > > from .i18n import _ > -from . import error > +from . import ( > + error, > + util, > +) > > def gnugetopt(args, options, longoptions): > """Parse options mostly like getopt.gnu_getopt. > @@ -91,6 +94,10 @@ def fancyopts(args, options, state, gnu= > short += ':' > if oname: > oname += '=' > + else: > + prefix = '--' + oname + '=' > + if any(a.startswith(prefix) for a in args): > + oname += '=' > if short: > shortlist += short > if name: > @@ -121,7 +128,10 @@ def fancyopts(args, options, state, gnu= > elif t is type([]): > state[name].append(val) > elif t is type(None) or t is type(False): > - state[name] = True > + if val: > + state[name] = util.parsebool(val) > + else: > + state[name] = True > > # return unparsed args > return args > diff --git a/tests/test-update-branches.t b/tests/test-update-branches.t > --- a/tests/test-update-branches.t > +++ b/tests/test-update-branches.t > @@ -379,3 +379,14 @@ Test experimental revset support > > $ hg log -r '_destupdate()' > 2:bd10386d478c 2 (no-eol) > + > +Test that boolean flags allow --flag=false specification to override [defaults] > + $ cat >> $HGRCPATH <<EOF > + > [defaults] > + > update = --check > + > EOF > + $ hg co 2 > + abort: uncommitted changes > + [255] > + $ hg co --check=no 2 > + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org <mailto:Mercurial-devel@mercurial-scm.org> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel <https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel> >
On Thu, 18 Aug 2016 12:13:33 -0400, Augie Fackler wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1471534453 14400 > # Thu Aug 18 11:34:13 2016 -0400 > # Node ID 9ec3aad0bf74ad1c25e87bcd0f536896d1c355ca > # Parent 8ed7fdc716ffc95a4062875eafc2739697122fbc > flags: allow specifying --boolean-flag=(true|false) on the command line (BC) > > This makes it much easier to enable some anti-foot-shooting features > (like update --check) by default, because now all boolean flags can be > explicitly disabled on the command line without having to use HGPLAIN > or similar. I understand this will be somewhat useful, but I'm not a big fan of introducing less common parsing rules. > --- a/mercurial/fancyopts.py > +++ b/mercurial/fancyopts.py > @@ -10,7 +10,10 @@ from __future__ import absolute_import > import getopt > > from .i18n import _ > -from . import error > +from . import ( > + error, > + util, > +) > > def gnugetopt(args, options, longoptions): > """Parse options mostly like getopt.gnu_getopt. > @@ -91,6 +94,10 @@ def fancyopts(args, options, state, gnu= > short += ':' > if oname: > oname += '=' > + else: > + prefix = '--' + oname + '=' > + if any(a.startswith(prefix) for a in args): > + oname += '=' args may contain a value looks like --option=. $ hg up -t --check=false --check > - state[name] = True > + if val: > + state[name] = util.parsebool(val) this should abort if val isn't a boolean literal. > --- a/tests/test-update-branches.t > +++ b/tests/test-update-branches.t > @@ -379,3 +379,14 @@ Test experimental revset support > > $ hg log -r '_destupdate()' > 2:bd10386d478c 2 (no-eol) > + > +Test that boolean flags allow --flag=false specification to override [defaults] > + $ cat >> $HGRCPATH <<EOF > + > [defaults] > + > update = --check > + > EOF > + $ hg co 2 > + abort: uncommitted changes > + [255] > + $ hg co --check=no 2 Perhaps this would be parsed as --check '--check=no'.
On Mon, Aug 22, 2016 at 9:41 AM, Yuya Nishihara <yuya@tcha.org> wrote: >> + abort: uncommitted changes >> + [255] >> + $ hg co --check=no 2 > > Perhaps this would be parsed as --check '--check=no'. Per an IRC conversation, I'm going to attempt to rework this to be either --check or --no-check, which is more in line with some GNU utilities and shouldn't require reimplementation of a big part of getopt.
Patch
diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py --- a/mercurial/fancyopts.py +++ b/mercurial/fancyopts.py @@ -10,7 +10,10 @@ from __future__ import absolute_import import getopt from .i18n import _ -from . import error +from . import ( + error, + util, +) def gnugetopt(args, options, longoptions): """Parse options mostly like getopt.gnu_getopt. @@ -91,6 +94,10 @@ def fancyopts(args, options, state, gnu= short += ':' if oname: oname += '=' + else: + prefix = '--' + oname + '=' + if any(a.startswith(prefix) for a in args): + oname += '=' if short: shortlist += short if name: @@ -121,7 +128,10 @@ def fancyopts(args, options, state, gnu= elif t is type([]): state[name].append(val) elif t is type(None) or t is type(False): - state[name] = True + if val: + state[name] = util.parsebool(val) + else: + state[name] = True # return unparsed args return args diff --git a/tests/test-update-branches.t b/tests/test-update-branches.t --- a/tests/test-update-branches.t +++ b/tests/test-update-branches.t @@ -379,3 +379,14 @@ Test experimental revset support $ hg log -r '_destupdate()' 2:bd10386d478c 2 (no-eol) + +Test that boolean flags allow --flag=false specification to override [defaults] + $ cat >> $HGRCPATH <<EOF + > [defaults] + > update = --check + > EOF + $ hg co 2 + abort: uncommitted changes + [255] + $ hg co --check=no 2 + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved