Use repos.display_rev for revision numbers that are displayed in templates

[trbs <trbs@…>]

When using version control systems that are based on hashes of changesets, like Mercurial or GIT the revision numbers are often shorted or alternatively formatted so that the user doesn't have to see the large hex.

Therefor I've added display_rev on several places in Bitten so that templates always display the formatted revision number.

For example in Mercurial the revision number now looks like:

 386:b9d1f6a7318b 

Instead of:

 b9d1f6a7318b3300150d951a478448a3d03fb9df

Please look at the code and commit in: ​http://bitbucket.org/trbs/bitten/changeset/338389921ce0

The patch is also attached :)

[hodgestar]

Looks good.

[trbs <trbs@…>]

There is one addition for the timeline at: ​http://bitbucket.org/trbs/bitten/changeset/0a0bce41190a

[osimons]

Makes sense and looks good. A complete refreshed patch for a final "test & apply" will see that we can get this committed now before 0.6.

[Felix Schwarz <felix.schwarz@…>]

When porting the patch to the latest 0.6 branch, I found some problems – The biggest one is that it causes 4 unit test errors because the Mock request does not have an attribute 'display_rev'. I verified that I ported the patch correctly by running the test suite from a clone of trbs' repository and I see even more failures there.

The problem should be relatively simple to fix but the patch can't go in as it is right now.

[Felix Schwarz <felix.schwarz@…>]

Updated patch (latest 0.6), still causes some test failures

[osimons]

Use normalize_rev() instead + fix tests.

[osimons]

Thanks Felix!

Looking at 0.11 and 0.12 tagged versions of trac.versioncontrol.api, I see that display_rev is a newer attribute available in 0.12. For 0.11 there is something called short_rev. However, both just do return self.normalize_rev(rev), so that is perhaps also what we should call directly - normalize_rev(rev) instead of repos.display_rev(rev).

Attached is a patch that uses this new call + updates the fake Mock repository implementations in tests to support the new method where needed.

Testing needed.

[Felix Schwarz <felix.schwarz@…>]

fix invalid method name - obviously the tests are not covering all of the code…

[Felix Schwarz <felix.schwarz@…>]

So checked the patch against latest Trac 0.11 and I don't see any problems besides the one mentioned in comment 7.

However I'm also not seeing any change with the Mercurial plugin. I'm just assuming that it makes a difference for some people. I always saw the short hashes with trbs showed in the description. This is with hg 1.2 on CentOS 4 and 1.6 on Fedora 13. Maybe this also depends on the Mercurial Plugin? for trac?

[osimons]

No, the tests don't really have 100% coverage... The timeline provider is not currently tested at all, and I don't think I hit 'Timeline' on my site while testing the patch visually. Anyway, thanks for discovering. I've updated the full patch.

As for what is displayed, I see the Trac project repository browser now also displays some Mercurial development repositories. They use revision numbers like 7132:d441086e653b everywhere. Is that what you get from Bitten now?

/me really needs to get a Mercurial development project running...

[Felix Schwarz <felix.schwarz@…>]

Replying to osimons:

As for what is displayed, I see the Trac project repository browser now also displays some Mercurial development repositories. They use revision numbers like 7132:d441086e653b everywhere. Is that what you get from Bitten now?

Exactly – however I saw exactly the same format also before…

/me really needs to get a Mercurial development project running...

If you need some nice features for motivation: easy hunk-level commits (hg record), move your stuff aside to fix a small issue (hg shelve) and keep patches for an open source project where you can't commit yourself (hg patch queue). :-)

[osimons]

If the patch doesn't make any difference to the user experience, I don't really see the value of applying it. Anyway, suggesting we push this issue to a bit later.

[trbs <bitten@…>]

Sorry I've been on vacation last view weeks.

So it does show normalized_rev numbers on Trac 0.11 ?

I've been using Trac 0.12 (with several patches / quickfixes to bitten to make that work) and in my environment the user experience was greatly enhanced by using normalized revision numbers instead of the full hashes.

If I remember correctly the only place where otherwise normalized version numbers where shown is in the timeline..

[Felix Schwarz <felix.schwarz@…>]

Replying to trbs <bitten@…>:

So it does show normalized_rev numbers on Trac 0.11 ?

Yes I think so. Just have a look at the ​build page of pycerberus. My trac does not contain any of the patches mentioned in this ticket. This is with 0.11 though, I could check if 0.12 is different.

Which DVCS are you using? With version exactly?

[trbs <bitten@…>]

That looks fine indeed, interestingly links are also in normalized format as well as the link urls.

I'm using Trac 0.12 with Trac Mercurial? 0.12.0.23dev-r9946

[trbs <bitten@…>]

Running hg --version Mercurial Distributed SCM (version 1.6.3+2-b10b07a821c3)

[osimons]

Oh. Seems I misunderstood. This is now the conclusions it seems:

• Trac 0.11 users will not see the change, all things as before
• Trac 0.12 users will see simplified revisions for those back-ends that support it, it is nice, it is useful, and everyone cheers

Back on track then.

[trbs <bitten@…>]

Hopefully somebody can confirm my findings with Trac 0.12 :)

I've taken a quick look at Trac Mercurial? plugin this afternoon and could not find why it would make a difference between 0.11 and 0.12.

Originally I had chosen display_rev because of the comments on the API

Return a representation of a revision in the repos for displaying to the user.

vs

Return a (unique) canonical representation of a revision.

Thinking that it might be better because other back-ends could have differences between normalize_rev and display_rev.

Lastly should bitten/web_ui.py: "display_rev = repos.normalize_revv(rev)" not be surrounded by a try-catch ? just in case ?

I had problems because I migrated from SVN to Mercurial. Thus having svn revision id's which it wants to lookup in mercurial raising a Change Set Not Found? exception.

[osimons]

Committed in [912]. Thanks both for work on this issue.