[PATCH] bookmarks: simplify code

[Augie Fackler]

Looks reasonable, queued.

On Oct 17, 2012, at 12:21 PM, Kevin Bullock <kbullock+mercurial at ringworld.org> wrote:

> # HG changeset patch
> # User Kevin Bullock <kbullock at ringworld.org>
> # Date 1350494123 18000
> # Node ID 4dc03350b9e8e0e6e25e3796465d0d65463d16da
> # Parent  9469bbd2f3a1e39c086383f3fe1543b801991b6f
> bookmarks: simplify code
> 
> Remove some unnecessary return statements and collect some checks into
> one place. As requested by Thomas Arendsen Hein <thomas at intevation.de>.
> 
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -813,19 +813,18 @@ def bookmark(ui, repo, mark=None, rev=No
>         raise util.Abort(_("--rev is incompatible with --delete"))
>     if rename and rev:
>         raise util.Abort(_("--rev is incompatible with --rename"))
> +    if mark is None and (delete or rev):
> +        raise util.Abort(_("bookmark name required"))
> 
>     if delete:
> -        if mark is None:
> -            raise util.Abort(_("bookmark name required"))
>         if mark not in marks:
>             raise util.Abort(_("bookmark '%s' does not exist") % mark)
>         if mark == repo._bookmarkcurrent:
>             bookmarks.setcurrent(repo, None)
>         del marks[mark]
>         bookmarks.write(repo)
> -        return
> -
> -    if rename:
> +
> +    elif rename:
>         if mark is None:
>             raise util.Abort(_("new bookmark name required"))
>         mark = checkformat(mark)
> @@ -837,9 +836,8 @@ def bookmark(ui, repo, mark=None, rev=No
>             bookmarks.setcurrent(repo, mark)
>         del marks[rename]
>         bookmarks.write(repo)
> -        return
> -
> -    if mark is not None:
> +
> +    elif mark is not None:
>         mark = checkformat(mark)
>         if inactive and mark == repo._bookmarkcurrent:
>             bookmarks.setcurrent(repo, None)
> @@ -854,17 +852,14 @@ def bookmark(ui, repo, mark=None, rev=No
>         bookmarks.write(repo)
>         return
> 
> -    if mark is None:
> -        if rev:
> -            raise util.Abort(_("bookmark name required"))
> +    else: # mark is None
>         if len(marks) == 0:
>             ui.status(_("no bookmarks set\n"))
> -        if inactive:
> +        elif inactive:
>             if not repo._bookmarkcurrent:
>                 ui.status(_("no active bookmark\n"))
>             else:
>                 bookmarks.setcurrent(repo, None)
> -            return
>         else:
>             for bmark, n in sorted(marks.iteritems()):
>                 current = repo._bookmarkcurrent
> @@ -879,7 +874,6 @@ def bookmark(ui, repo, mark=None, rev=No
>                     ui.write(" %s %-25s %d:%s\n" % (
>                         prefix, bmark, repo.changelog.rev(n), hexfn(n)),
>                         label=label)
> -        return
> 
> @command('branch',
>     [('f', 'force', None,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel