Submitter | Sean Farley |
---|---|
Date | June 21, 2017, 12:29 a.m. |
Message ID | <03591d8a6d8247d35bbd.1498004964@1.0.0.127.in-addr.arpa> |
Download | mbox | patch |
Permalink | /patch/21555/ |
State | Accepted |
Headers | show |
Comments
On Tue, Jun 20, 2017 at 5:29 PM, Sean Farley <sean@farley.io> wrote: > # HG changeset patch > # User Sean Farley <sean@farley.io> > # Date 1497162778 25200 > # Sat Jun 10 23:32:58 2017 -0700 > # Branch bm-refactor > # Node ID 03591d8a6d8247d35bbdc93cebce1757755ca463 > # Parent 6d79e9109908c2cb468c9eeaf8869aa1926fbea8 > commands: move checkformat to bookmarks module > Queued parts 1 and 2. > > commands.bookmark has grown quite large with two closures already. Let's > split this up (and in the process allow extensions to override the > default behavior). > > diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py > index db01ef9..948bfbe 100644 > --- a/mercurial/bookmarks.py > +++ b/mercurial/bookmarks.py > @@ -17,10 +17,11 @@ from .node import ( > from . import ( > encoding, > error, > lock as lockmod, > obsolete, > + scmutil, > txnutil, > util, > ) > > def _getbkfile(repo): > @@ -620,5 +621,17 @@ def validdest(repo, old, new): > elif repo.obsstore: > return new.node() in obsolete.foreground(repo, [old.node()]) > else: > # still an independent clause as it is lazier (and therefore > faster) > return old.descendant(new) > + > +def checkformat(repo, mark): > + """return a valid version of a potential bookmark name > + > + Raises an abort error if the bookmark name is not valid. > + """ > + mark = mark.strip() > + if not mark: > + raise error.Abort(_("bookmark names cannot consist entirely of " > + "whitespace")) > + scmutil.checknewlabel(repo, mark, 'bookmark') > + return mark > diff --git a/mercurial/commands.py b/mercurial/commands.py > index a56cc76..abb42f7 100644 > --- a/mercurial/commands.py > +++ b/mercurial/commands.py > @@ -955,18 +955,10 @@ def bookmark(ui, repo, *names, **opts): > rev = opts.get('rev') > delete = opts.get('delete') > rename = opts.get('rename') > inactive = opts.get('inactive') > > - def checkformat(mark): > - mark = mark.strip() > - if not mark: > - raise error.Abort(_("bookmark names cannot consist entirely > of " > - "whitespace")) > - scmutil.checknewlabel(repo, mark, 'bookmark') > - return mark > - > def checkconflict(repo, mark, cur, force=False, target=None): > if mark in marks and not force: > if target: > if marks[mark] == target and target == cur: > # re-activating a bookmark > @@ -1037,11 +1029,11 @@ def bookmark(ui, repo, *names, **opts): > tr = repo.transaction('bookmark') > if not names: > raise error.Abort(_("new bookmark name required")) > elif len(names) > 1: > raise error.Abort(_("only one new bookmark name > allowed")) > - mark = checkformat(names[0]) > + mark = bookmarks.checkformat(repo, names[0]) > if rename not in marks: > raise error.Abort(_("bookmark '%s' does not exist") > % rename) > checkconflict(repo, mark, cur, force) > marks[mark] = marks[rename] > @@ -1050,11 +1042,11 @@ def bookmark(ui, repo, *names, **opts): > del marks[rename] > elif names: > tr = repo.transaction('bookmark') > newact = None > for mark in names: > - mark = checkformat(mark) > + mark = bookmarks.checkformat(repo, mark) > if newact is None: > newact = mark > if inactive and mark == repo._activebookmark: > bookmarks.deactivate(repo) > return > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
Patch
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py index db01ef9..948bfbe 100644 --- a/mercurial/bookmarks.py +++ b/mercurial/bookmarks.py @@ -17,10 +17,11 @@ from .node import ( from . import ( encoding, error, lock as lockmod, obsolete, + scmutil, txnutil, util, ) def _getbkfile(repo): @@ -620,5 +621,17 @@ def validdest(repo, old, new): elif repo.obsstore: return new.node() in obsolete.foreground(repo, [old.node()]) else: # still an independent clause as it is lazier (and therefore faster) return old.descendant(new) + +def checkformat(repo, mark): + """return a valid version of a potential bookmark name + + Raises an abort error if the bookmark name is not valid. + """ + mark = mark.strip() + if not mark: + raise error.Abort(_("bookmark names cannot consist entirely of " + "whitespace")) + scmutil.checknewlabel(repo, mark, 'bookmark') + return mark diff --git a/mercurial/commands.py b/mercurial/commands.py index a56cc76..abb42f7 100644 --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -955,18 +955,10 @@ def bookmark(ui, repo, *names, **opts): rev = opts.get('rev') delete = opts.get('delete') rename = opts.get('rename') inactive = opts.get('inactive') - def checkformat(mark): - mark = mark.strip() - if not mark: - raise error.Abort(_("bookmark names cannot consist entirely of " - "whitespace")) - scmutil.checknewlabel(repo, mark, 'bookmark') - return mark - def checkconflict(repo, mark, cur, force=False, target=None): if mark in marks and not force: if target: if marks[mark] == target and target == cur: # re-activating a bookmark @@ -1037,11 +1029,11 @@ def bookmark(ui, repo, *names, **opts): tr = repo.transaction('bookmark') if not names: raise error.Abort(_("new bookmark name required")) elif len(names) > 1: raise error.Abort(_("only one new bookmark name allowed")) - mark = checkformat(names[0]) + mark = bookmarks.checkformat(repo, names[0]) if rename not in marks: raise error.Abort(_("bookmark '%s' does not exist") % rename) checkconflict(repo, mark, cur, force) marks[mark] = marks[rename] @@ -1050,11 +1042,11 @@ def bookmark(ui, repo, *names, **opts): del marks[rename] elif names: tr = repo.transaction('bookmark') newact = None for mark in names: - mark = checkformat(mark) + mark = bookmarks.checkformat(repo, mark) if newact is None: newact = mark if inactive and mark == repo._activebookmark: bookmarks.deactivate(repo) return