#13313 closed defect (fixed)
ChangeLog macro (more) link does not work when you specify a repository
| Reported by: | Ian Lewis | Owned by: | Ryan J Ollos |
|---|---|---|---|
| Priority: | normal | Component: | ChangeLogMacro |
| Severity: | minor | Keywords: | |
| Cc: | Trac Release: |
Description
If you specify a repository to the ChangeLog() macro it creates a (more) link at the bottom of the listing. That link has an incorrect href when you specify the repository to report against.
The patch below corrects the problem.
@@ -153,7 +153,9 @@ message = _remove_p(format_to_html( self.env, context, change.message, escape_newlines=True)) out.write('\n<dd>\n%s\n</dd>' % message) - out.write(html.small(html.a(_("(more)"), href=req.href.log(path)))) + if reponame: + reponame = reponame + '/' + out.write(html.small(html.a(_("(more)"), href=req.href.log('%s%s' % (reponame, path))))) out.write('\n</dl>\n</div>') out.write('\n<p>') # re-open surrounding paragraph return out.getvalue()
Attachments (0)
Change History (8)
comment:1 Changed 8 years ago by
| Status: | new → accepted |
|---|
comment:2 Changed 8 years ago by
comment:3 follow-ups: 4 5 7 Changed 8 years ago by
The convention in Trac is to use the repository name in the path. For example, /path in repos1 is /repos1/path (see TracLinks#VersionControlrelatedlinks). I modified the plugin to use the convention, but it should be backward compatible with the repos1:/trunk pattern.
Let me know if you spot any issues. I will leave the ticket open for adding test coverage and further refactoring in the coming days.
comment:4 Changed 8 years ago by
Replying to Ryan J Ollos:
The convention in Trac is to use the repository name in the path.
This sounds good to me. I have often wished all plugins followed this convention, instead of having a separate repo parameter of some kind.
I will test the patch here.
comment:5 follow-up: 6 Changed 8 years ago by
Replying to Ryan J Ollos:
I modified the plugin to use the convention, but it should be backward compatible with the
repos1:/trunkpattern.
The update works well on our site. I saw no issues with either the new or old repository path syntax.
Update: Ignore the following. I see in the link you supplied that you have a clean resolution to the issue: the repository name takes precedence.
I prefer the new syntax, by quite a lot, though I did think of one thing I have never thought about before.
The old syntax allowed for a distinction between the repository name and a path of the same name in the default repository. For example, if you had a repository called ARep and the default repository had a path /ARep the new syntax looks identical for a file at the root of ARep or under the folder /ARep in the default repository: /ARep/SomeFile.
This is really a general Trac issue, not an issue with this macro. Most likely you have some resolution, though I do not know what it is.
I have never thought about this issue because we never use a default repository. So, for our usage, the first part of the path is always a repository name. And, that works very nicely.
comment:6 Changed 8 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | accepted → closed |
Replying to Ian Lewis:
The old syntax allowed for a distinction between the repository name and a path of the same name in the default repository. For example, if you had a repository called
ARepand the default repository had a path/ARepthe new syntax looks identical for a file at the root ofARepor under the folder/ARepin the default repository:/ARep/SomeFile.This is really a general Trac issue, not an issue with this macro. Most likely you have some resolution, though I do not know what it is.
The case is discussed in TracLinks#VersionControlrelatedlinks.
comment:7 Changed 8 years ago by
Replying to Ryan J Ollos:
Thank you for fixing this issue so fast. - Ian



In 16913: