minor issues with "hg bookmark"

[Kevin Bullock]

On Oct 9, 2012, at 5:01 AM, Thomas Arendsen Hein wrote:

> Hi Kevin!
> 
>> bookmarks: deactivate current bookmark if no name is given
> 
> While looking at the code I saw some minor things:
> 
> - Renaming bookmark does not check all things that marking does,
>  e.g. newlines, whitespace and branch name collision.
> 
> - Incompatible options are not catched, e.g. something like
> 
>    if delete and rename:
>        raise util.Abort(_("--delete and --rename are incompatible"))
> 
>  (and some more)
> 
> - There are far too many return statements compared to other parts
>  of the code. I suggest dropping them where not needed and
>  replacing "if xx:" with "elif xx:" elsewhere, or even simply
>  "else:" in the case of "if mark is None:".
>  Of course this is a matter of taste, the explicit returns and
>  checks could help prevent mistakes, but as other parts of the code
>  don't do this, it feels too much here.
> 
> Can you send patches for this for me to push to crew (not stable)?

Sure, I'll take a look at it.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121009/5bda22ee/attachment.html>