[PATCH 1 of 2] graph: in hgrc specify line width for main branch

[Constantine]

Martin, can it be done without history editing, e.g. in third commit?

2012/2/6 Martin Geisler <mg at aragost.com>

> Constantine <theaspect at gmail.com> writes:
>
> > Martin, Linus said "Just for fun". Are You think resending simple
> > patch eight times funny?
>
> Sorry about that -- we (core developers) often forget that it's not fun
> to submit patches again and again... some people really appreciate the
> feedback, but others have better things to do.
>
> Overall, we want really pretty code in Mercurial and so the bar is a
> little high for getting patches accepted.
>
> > If my contribution worth somesing, you can fix it yourself, otherwise
> > throw it away. But bear in mind, "graph" visualisation in hgweb
> > completely broken: try to compare high-branched tree with tortoisehg's
> > version.
> >
> > 2012/2/2 Martin Geisler <mg at aragost.com>
> >
> >> Constantine Linnick <theaspect at gmail.com> writes:
> >>
> >> > # HG changeset patch
> >> > # User Constantine Linnick <theaspect at gmail.com>
> >> > # Date 1327235726 -25200
> >> > # Node ID 537733973603cb4f974f01aecd51923da17e314f
> >> > # Parent  878bc4a62a735a1624e9b69c672d5fb5074d4855
> >> > graph: in hgrc specify line width for main branch
> >> >
> >> > You can specify width to visually distinguish main branch (trunk)
> >> > on hgweb's graph page. Settings format is branch_name.width = value,
> >> > where width in px e.g.:
> >> > [graph]
> >> > default.width = 3
> >> >
> >> > diff -r 878bc4a62a73 -r 537733973603 mercurial/graphmod.py
> >> > --- a/mercurial/graphmod.py   Thu Jan 19 14:31:05 2012 -0600
> >> > +++ b/mercurial/graphmod.py   Sun Jan 22 19:35:26 2012 +0700
> >> > @@ -18,6 +18,7 @@
> >> >  """
> >> >
> >> >  from mercurial.node import nullrev
> >> > +import re
> >> >
> >> >  CHANGESET = 'C'
> >> >
> >> > @@ -67,7 +68,7 @@
> >> >          parents = set([p.rev() for p in ctx.parents() if p.node() in
> >> include])
> >> >          yield (ctx.rev(), CHANGESET, ctx, sorted(parents))
> >> >
> >> > -def colored(dag):
> >> > +def colored(dag, repo):
> >> >      """annotates a DAG with colored edge information
> >> >
> >> >      For each DAG node this function emits tuples::
> >> > @@ -83,6 +84,21 @@
> >> >      seen = []
> >> >      colors = {}
> >> >      newcolor = 1
> >> > +    config = dict()
> >>
> >> There is another empty dict just above and it uses {}. I'm not sure
> >> which I prefer myself, but it's important to be consistent. You
> >> should therefore use {} here too so that you keep the coding style
> >> consistent.
> >
> > Here:
> >
> http://www.selenic.com/pipermail/mercurial-devel/2012-January/036816.html
> > Mark said "No bug fixes, no spelling fixes, no coding style fixes, no
> > whitespace fiddling, nothing."
>
> I meant that I think you should use '{}' above to keep the overall style
> more consistent. That's all.
>
> (Btw, his name is Matt, not Mark.)
>
Just typo

>
> >> +    for key, val in repo.ui.configitems('graph'):
> >> > +        if '.' not in key:
> >> > +            continue
> >> > +        branch, setting = key.rsplit('.',1)
> >>
> >> We use PEP 8 formatting in Mercurial. Please remember to run the full
> >> test suite before submitting patches -- test-check-code-hg.t should
> >> have caught the missing spaces around ','.
> >
> > I've run All tests. Your particular test case:
> >
> > ~/workspace/mercurial-repo/tests$ ./run-tests.py test-check-code-hg.t
> > .
> > # Ran 1 tests, 0 skipped, 0 failed.
>
> Okay, I'm sorry the check-code script didn't catch this mistake.
>
> >> > +                (setting == "width" and not re.match('[0-9]{1,2}',
> >> val))):
> >>
> >> Please don't use regular expressions for something that can be
> >> checked with simpler string functions:
> >
> > I test not digit, I test digit between 0 and 99. Small regexp simplier
> > then two tests
>
> I did see that you check for a number in the range 0..100. I think
> that's best done with a simple
>
>  0 < int(val) < 100
>
> Here I would actually just check that 0 < int(val) and avoid the
> arbitrary restriction on the line width. Unless it isn't arbitrary? I
> guess the graph look really weird with a 100px wide line. I guess it
> also looks pretty bad with a 30px line width?
>
> Ok

> --
> Martin Geisler
>
> aragost Trifork
> Professional Mercurial support
> http://www.aragost.com/mercurial/customer-projects/
>



-- 
With Best Regards,
Constantine
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20120206/fd8a526f/attachment.html>