Opened 2 years ago

Closed 2 years ago

# ChangeLog macro (more) link does not work when you specify a repository

Reported by: Owned by: Ian Lewis Ryan J Ollos normal ChangeLogMacro minor

### 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()


### comment:1 Changed 2 years ago by Ryan J Ollos

Status: new → accepted

### comment:2 Changed 2 years ago by Ryan J Ollos

In 16913:

TracChangeLog 0.5dev: Make compatible with Trac 1.3.x and fix "(more)" link

Refs #13313.

### comment:3 follow-ups:  4  5  7 Changed 2 years ago by Ryan J Ollos

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.

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

### comment:4 in reply to:  3 Changed 2 years ago by Ian Lewis

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 in reply to:  3 ; follow-up:  6 Changed 2 years ago by Ian Lewis

Replying to Ryan J Ollos:

I modified the plugin to use the convention, but it should be backward compatible with the repos1:/trunk pattern.

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.

Last edited 2 years ago by Ian Lewis (previous) (diff)

### comment:6 in reply to:  5 Changed 2 years ago by Ryan J Ollos

Resolution: → fixed 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 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.

### comment:7 in reply to:  3 Changed 2 years ago by Ian Lewis

Replying to Ryan J Ollos:

Thank you for fixing this issue so fast. - Ian

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

### comment:8 Changed 2 years ago by Ryan J Ollos

No problem. Thanks for reporting and testing!

### Modify Ticket

Change Properties