Opened 11 years ago

Closed 7 years ago

Last modified 6 years ago

#9175 closed defect (fixed)

[PATCH] BackLinksMenuMacro was not working on TRAC 0.12.x

Reported by: Red Owned by: Ryan J Ollos
Priority: normal Component: BackLinksMacro
Severity: major Keywords:
Cc: Trac Release: 0.12


Fairly new TRAC 0.12 install. Although I was able to get BackLinksMacro.5 working, I was unable to get BackLinksMenuMacro working (it was only partially working, but not displaying the actual web page links.).

Code inspection between the 2 macros pointed to a few areas where there has been some changes. Gradually bringing code across from BackLinksMacro --> BackLinksMenuMacro resulted in a stable and working copy.

Submitting this ticket and trying to check in the patch has already proven to be more complicated/difficult than anticiapted for such a simple set of changes. There are a few spurious changes in the patch that could be removed, but I was trying to keep the code "lined up" with BackLinksMacro for easier 'diffing'.

Attachments (2)

BackLinksMenu_TRAC0.12_patch.diff (3.1 KB) - added by Red 11 years ago.
PATCH file for making BackLinksMenuMacro work for TRAC 0.12 (3.5 KB) - added by Ryan J Ollos 10 years ago.
Previously attached to project wiki page.

Download all attachments as: .zip

Change History (10)

Changed 11 years ago by Red

PATCH file for making BackLinksMenuMacro work for TRAC 0.12

comment:1 Changed 11 years ago by osimons

I just came across this latest patch by accident, and thought I'd just mention two things I spotted:

  1. The patch turns all code (including code by other copyright holder) into GPL3? The plugin should of course have some stated license, but I think the original author should decide that. And, if the new patch is only be offered as GPL3 then that should be clearly stated as applying it effectively infects the plugin and turns it into GPL3 anyway.
  2. The plugin SQL is badly written as the escaping should be left to the DB backend. There are plenty of examples in the Trac code for how you just use %s placeholders in the SQL string, and pass variables for substitution along with the sql: cursor.execute(sql, args)

comment:2 Changed 11 years ago by osimons

Oh, about the SQL changes: Forgot to mention that there is also a and db.like_escape() methods for inserting the LIKE syntax used by current DB backend. Again, search the Trac source code for examples.

comment:3 Changed 11 years ago by Red

Sorry about the GPL3 -- i added that because it was in trapanator's BackLinksMacro.5 I don't think it's necessary either, but that' sup to the original author.

I have no knowledge of SQL, sorry. Once again, just copied/pasted what I found in the working BackLinksMacro. If there is a better way to do this, that's cool....but I'm not the guy to do it, unfortunately. I can lend a hand by running tests on any updated patches, though.

Should I submit an updated patch without the GPL 3 added?

comment:4 in reply to:  3 Changed 11 years ago by osimons

Replying to mizraith:

Sorry about the GPL3

I have no knowledge of SQL, sorry.

No need for apologies. I just left the notes for the author of the plugin to decide. No need for updated patch, the plugin author can no doubt fix things as needed. I don't use the plugin nor do I have any particular interest in it, so I just voiced my opinion about some of the code.

Changed 10 years ago by Ryan J Ollos

Attachment: added

Previously attached to project wiki page.

comment:5 Changed 10 years ago by Ryan J Ollos

Component: BackLinksMenuMacroBackLinksMacro

The functionality of BackLinksMenuMacro is being integrated into the BackLinksMacro.

comment:6 Changed 10 years ago by Ryan J Ollos

(In [11969]) Refs #5903, #9175, #9503:

  • Merged codebases for BackLinksMacro and BackLinksMenuMacro.
  • Modified SQL query to utilize Trac's database agnostic API. Possible matches are captured with a LIKE clause, and then filtered with a regex in the Python code.

comment:7 Changed 10 years ago by Ryan J Ollos

Owner: changed from Trap to Ryan J Ollos

I'm not sure that I'll do much more work with this macro since it is GPL licensed, but I'd appreciate any feedback on the approach I took with using the LIKE clause in the query and then filtering the results in Python. I couldn't see any other way to do it without using a REGEXP in the query, and the Trac DB API doesn't have an abstraction for REGEXP as far as I could see.

Anyway, I think the macros are at least in better shape than they were. I might take the additional step of merging in the TicketBackLinksMacro. The new package could use some more work if anyone wants to take over maintenance. I haven't been able to reach trapanator.

Feedback from testing is also appreciated.

comment:8 Changed 7 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

In 15134:

7.0dev: Pass env rather than db object

This will make it easier to eventually adapt the code
to the Trac 1.0 DB API.

Fixes #9175.

Modify Ticket

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

Add Comment

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

Note: See TracTickets for help on using tickets.