Opened 18 years ago
Last modified 9 years ago
#4612 new enhancement
Changeset URL to contain the revision in the query string.
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | normal | Milestone: | next-major-releases |
Component: | version control | Version: | 0.10.3 |
Severity: | normal | Keywords: | revision patch |
Cc: | Branch: | ||
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
I’m using bzr as version control backend provided by the trac+bzr plugin. Since each branch has its own revision numbers, the revisions as seen by trac have the format:
path/to/branch,revno
ChangesetModule.match_request will match 'path' to the revision and 'to/branch,revno' as the path. trac+bzr works around this problem by URL escaping the revision string but the result is highly unreadable and not at all user-friendly.
Attached is a very simple patch I’m using to make trac gracefully handle the above scenario. I have added a configuration switch for the new format because I expect some people would like to keep the old URLs valid.
The solution is simply to change the changeset URLs from:
/changeset/revision/path
to:
/changeset/path?new=revision
The latter is also totally consistent with URLs of log and browser:
/log/path?rev=old_rev&stop_rev=new_rev /browser/path?rev=revision
The patch also allows a new changeset notation:
changeset:path@revision
Clearly the changeset:revision/path
notation cannot be used when revision can contain a slash.
Attachments (3)
Change History (10)
by , 18 years ago
Attachment: | trac-0.10.3-changeset.revision_in_querystring.patch added |
---|
follow-up: 2 comment:1 by , 18 years ago
comment:2 by , 18 years ago
Replying to cboos:
Clearly the
changeset:revision/path
notation cannot be used when revision can contain a slash.Well, you not only have that one, but also the r<rev>/<path> one, the [<rev>/<path>] one, the [<rev>:<rev2>/<path>] etc. So the problem is wider.
Yes, I knew about these but didn't touch them because of backwards compatibility. Actually these notations cannot be used with trac+bzr as it is now. I thought that if revisions have slashes, then changeset:path@revision
should be used instead. I guess I could also add support for [path@revision]
by patching get_wiki_syntax().
When you say bzr needs to have the branch path in the rev, doesn't the optional path restriction also need to be on that branch?
i.e. if you have rev=/stable/0.10,1232 and want to restrict to 'src', what do you write?
- /stable/0.10/src@/stable/0.10,1232
- src@/stable/0.10,1232
If it can be 1. and trac+bzr could determine where a branch stops, then we could simply use the normal syntax and write e.g.
[1232/stable/0.10/src]
.
Currently it is 1. and you are totally right that the normal syntax could also be used if trac+bzr was able to determine the branch base path. I was also thinking about this but the problem is that, for example, Repository.get_changeset() takes a string rev as parameter. trac+bzr needs both the path and the revno. What it would get in the case you suggest is only the revno and that is not enough. I already have some ideas how to solve that but they involve some refactoring in the api. I should better elaborate them in a separate ticket or in trac-dev, right?
by , 18 years ago
Attachment: | trac-0.10.3-changeset.revision_in_querystring.2.patch added |
---|
Old version of the patch actually broke the parsing of [<revision>/<path>]. Fixed now.
by , 18 years ago
Attachment: | trac-0.10.3-changeset.revision_in_querystring.3.patch added |
---|
This patches also the changeset entries in the timeline to support revision_in_querystring.
follow-up: 4 comment:3 by , 18 years ago
And what about using the double slash as a separator, if there must be a slash in the revision?
e.g. changeset:1232/stable/0.10//src
?
We would parse such a link the normal way, but there would be an extra step:
we look into the path for "" and if found, we append what's before to the revision (stable/0.10
- the branch part) and only keep what's after as the path (src
).
If think that with this approach, we could keep the existing regexps and simply have the simple post-processing step describe above.
And when we're about to generate a changeset link, we would take an extra step: checking if '/' is present in the revision, and if yes, add an extra '/' between the revision and the path, e.g.
href = formatter.href.changeset(revision, (None, '')['/' in revision], path)
This could be even simplified further using the Context API, as the logic above could be written in ChangesetContext.resource_href
, instead of in every href.changeset(...)
call.
comment:4 by , 18 years ago
Replying to cboos:
And what about using the double slash as a separator, if there must be a slash in the revision?
e.g.
changeset:1232/stable/0.10//src
? … And when we're about to generate a changeset link, we would take an extra step: checking if '/' is present in the revision, and if yes, add an extra '/' between the revision and the path, e.g.
I think that would work! Revsions returned from the backend must therefore have the format:
[revno/path/to/branch//] /changeset/revno/path/to/branch//
The above would display all changes in revno for branch. I don't like that the double slash in required in that case.
The log would create URLs like:
[revno/path/to/branch//path/to/branch/src/file.cpp]
What you suggested is nicer:
[revno/path/to/branch//src/file.cpp]
but it'll require additional pre and post processing.
I think the best solution would be not to change anything in changeset URL and Wiki format but instead pass both the path and the revno to the version control backend. Maybe let's wait and see what ideas we get from the trac-dev discussion on version control refactoring.
comment:6 by , 10 years ago
Owner: | removed |
---|
comment:7 by , 9 years ago
Keywords: | patch added |
---|
Replying to Peter Dimov <peter.dimov@calitko.org>:
Hi Peter,
I'm not really familiar with bzr specifics, so I very much appreciate that you bring those additional requirements on the table.
Ok, I think this makes sense. I originally chose the
changeset/rev/path
format in an attempt to get URLs looking more user friendly than the log and browser ones. I think there will be no big impact in switching to the URL format you proposed (while staying backward compatible, of course).Well, you not only have that one, but also the r<rev>/<path> one, the [<rev>/<path>] one, the [<rev>:<rev2>/<path>] etc. So the problem is wider.
When you say bzr needs to have the branch path in the rev, doesn't the optional path restriction also need to be on that branch?
i.e. if you have rev=/stable/0.10,1232 and want to restrict to 'src', what do you write?
If it can be 1. and trac+bzr could determine where a branch stops, then we could simply use the normal syntax and write e.g.
[1232/stable/0.10/src]
.