<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Oct 9, 2012, at 5:01 AM, Thomas Arendsen Hein wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Hi Kevin!<br><br><blockquote type="cite">bookmarks: deactivate current bookmark if no name is given<br></blockquote><br>While looking at the code I saw some minor things:<br><br>- Renaming bookmark does not check all things that marking does,<br>  e.g. newlines, whitespace and branch name collision.<br><br>- Incompatible options are not catched, e.g. something like<br><br>    if delete and rename:<br>        raise util.Abort(_("--delete and --rename are incompatible"))<br><br>  (and some more)<br><br>- There are far too many return statements compared to other parts<br>  of the code. I suggest dropping them where not needed and<br>  replacing "if xx:" with "elif xx:" elsewhere, or even simply<br>  "else:" in the case of "if mark is None:".<br>  Of course this is a matter of taste, the explicit returns and<br>  checks could help prevent mistakes, but as other parts of the code<br>  don't do this, it feels too much here.<br><br>Can you send patches for this for me to push to crew (not stable)?<br></div></blockquote><div><br></div><div>Sure, I'll take a look at it.</div></div><br><div>
<span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><span class="Apple-style-span" style="orphans: 2; text-align: -webkit-auto; text-indent: 0px; widows: 2; -webkit-text-decorations-in-effect: none; "><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和</div><div style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Kevin R. Bullock<br></div></span></span>
</div>
<br></body></html>