[PATCH 2 of 2] summary: indicate the current bookmark with '*'

[Matt Mackall]

On Tue, 2010-06-22 at 12:06 -0400, Cesar Mena wrote:
> On Tue, 22 Jun 2010 10:31:13 -0500, "Matt Mackall" <mpm at selenic.com>
> said:
> > On Mon, 2010-06-21 at 17:58 -0400, Cesar Mena wrote:
> > > # HG changeset patch
> > > # User Cesar Mena <cmena at pobox.com>
> > > # Date 1277155241 14400
> > > # Node ID 99431fbd97c234ee9166414d018fc189bb41c0ff
> > > # Parent  a5d8bceda56cad38b105d30a08d19569fcfbbb37
> > > summary: indicate the current bookmark with '*'
> > > 
> > > diff --git a/mercurial/commands.py b/mercurial/commands.py
> > > --- a/mercurial/commands.py
> > > +++ b/mercurial/commands.py
> > > @@ -3469,6 +3469,11 @@
> > >  
> > >      Returns 0 on success.
> > >      """
> > > +    def fmttag(t):
> > > +        if repo.tagtype(t) == 'bookmark':
> > > +            if repo._bookmarkcurrent == t:
> > > +                t = '*' + t
> > > +        return t
> > 
> > This is a HUGE layering violation. Nothing in the Mercurial directory
> > should have any awareness of anything in the hgext/ directory. Knowledge
> > of bookmarks should be entirely restricted to the bookmarks extension
> > itself.
> > 
> > And indeed, this code will give a traceback if the bookmark extension
> > isn't enabled (ie the default configuration).
> 
> Yes. However if the bookmark extension is not enabled, the tagtype won't
> be 'bookmark' and _bookmarkcurrent won't get used. I had hoped to make
> that a bit clearer by nesting the ifs. Ugly, scratch that. 
> 
> What about adding a 'summary' function in bookmarks (like mq.py does)
> and add an extra line to the summary output? 

Perhaps. Even better would be inventing a simple approach to make
summary easily extensible.

> > Also, we're in the middle of a code freeze for 1.6, please wait until
> > after July 1 to submit new feature patches.
> 
> Ok; thanks.
> 
> Cesar


-- 
Mathematics is the supreme nostalgia of our time.