#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 |
Description
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)
Change History (10)
Changed 13 years ago by
Attachment: | BackLinksMenu_TRAC0.12_patch.diff added |
---|
comment:1 Changed 13 years ago by
I just came across this latest patch by accident, and thought I'd just mention two things I spotted:
- 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.
- 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 13 years ago by
Oh, about the SQL changes: Forgot to mention that there is also a db.like()
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 follow-up: 4 Changed 13 years ago by
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 Changed 13 years ago by
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 12 years ago by
Attachment: | BackLinksMenu.py added |
---|
Previously attached to project wiki page.
comment:5 Changed 12 years ago by
Component: | BackLinksMenuMacro → BackLinksMacro |
---|
The functionality of BackLinksMenuMacro is being integrated into the BackLinksMacro.
comment:6 Changed 12 years ago by
(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 12 years ago by
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.
PATCH file for making BackLinksMenuMacro work for TRAC 0.12