Modify

Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#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 19 months ago by Ryan J Ollos

Status: newaccepted

comment:2 Changed 19 months 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 Changed 19 months 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 19 months ago by Ryan J Ollos (previous) (diff)

comment:4 in reply to:  3 Changed 19 months 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 ; Changed 19 months 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 19 months ago by Ian Lewis (previous) (diff)

comment:6 in reply to:  5 Changed 19 months ago by Ryan J Ollos

Resolution: fixed
Status: acceptedclosed

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.

The case is discussed in TracLinks#VersionControlrelatedlinks.

comment:7 in reply to:  3 Changed 19 months ago by Ian Lewis

Replying to Ryan J Ollos:

Thank you for fixing this issue so fast. - Ian

Last edited 19 months ago by Ryan J Ollos (previous) (diff)

comment:8 Changed 19 months ago by Ryan J Ollos

No problem. Thanks for reporting and testing!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.