Opened 7 years ago

# [Patch] The helppage field should transform the Keywords field into a link to an arbitrarily defined wiki page

Reported by: Owned by: Ryan J Ollos high KeywordSuggestPlugin normal link option Michael Renzmann 0.11

### Description

It would be a nice enhancement if the helpage parameters had behavior such that setting it to point to a wiki page would transform the Keywords field on the ticket form into a link to that page.

For example, I'd like to set helppage = /tags and have the Keywords field transformed to a link that point to the /tags page.

### comment:1 follow-ups:  2  3 Changed 7 years ago by Jay

I have actually made this enhancement. It doesn't specifically handle the case if the keywords = field is left blank, as I actually use it. however I will look at that modification also. It is using trac 11.5 with the patch in #4201 attached on 12/04/08

It is literally 5 characters different: on line 73 of keywordsuggest.py:

-                link = tag.a(href='%s' % helppage, target='blank')
+                link = tag.a(href='wiki/%s' % helppage, target='blank')

Then, in my trac.ini file, my helppage link is:

[keywordsuggest]
helppage = tags
keywords = tag1,tag2

this is line 73 of the merged patch to use the tags in keywordsuggest patch in #4201 (I used the whole file replacement 5.7kb option)

I don't think it's enough of a change to make a patch file at this point, but if I address the other issue, I will.

Question is, does this change belong with the tags patch in #4201, or with the main plugin code? I did it this way, so I could, instead if so desired, use a global keyword help page, since I am set up in a multi-environment trac configuration, and they all use the same basic keyword model. The downside, I suppose is that if you do want a wiki page, you have to use wiki/keywords or similiar, not a terrible burden

### comment:2 in reply to:  1 Changed 7 years ago by anonymous

Then, in my trac.ini file, my helppage link is:

[keywordsuggest]
helppage = tags
keywords = tag1,tag2

CORRECTION: sorry, I made a typo:
the helpage entry is:

• /ParentEnv/ProjectPage/tags

since I am multi-env and don't have all my paths set up. The above worked in a scenario where I had some funky apache stuff going on. Sorry about that, it should be:

[keywordsuggest]
helppage = /ParentEnv/ProjectPage/tags
keywords = tag1,tag2

### comment:3 in reply to:  1 Changed 7 years ago by Jay

Replying to yoheeb@gmail.com: I am not sure why the diff in the original looks like that (I just pasted) but I think it should be reversed (just get rid of wiki/). Either that or I need more coffee!:

-                link = tag.a(href='wiki/%s' % helppage, target='blank')
+                link = tag.a(href='%s' % helppage, target='blank')

### comment:4 Changed 6 years ago by Ryan J Ollos

I'll go ahead and make this change pending feedback on #8156.

### comment:5 Changed 6 years ago by Ryan J Ollos

Summary: The helppage field should transform the Keywords field into a link to an arbitrarily defined wiki page → [Patch] The helppage field should transform the Keywords field into a link to an arbitrarily defined wiki page

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

Priority: normal → high

### comment:7 Changed 6 years ago by Ryan J Ollos

Owner: changed from scratcher to Ryan J Ollos new → assigned

### comment:8 follow-up:  9 Changed 6 years ago by Aurélien Bompard

I have a (IMHO) better version of the patch, which works even when Trac is not at the root of the virtualhost. It uses Trac's own resource-to-link converter. I'll attach it to this ticket.

### comment:9 in reply to:  8 Changed 5 years ago by Ryan J Ollos

Cc: Steffen Hoffmann added; anonymous removed

I have a (IMHO) better version of the patch, which works even when Trac is not at the root of the virtualhost. It uses Trac's own resource-to-link converter. I'll attach it to this ticket.

It looks like your patch wouldn't allow, e.g. tags to be specified for the help page, since it is not in the wiki realm. If you take a look at the patch I'm about the commit and have any suggestions for improvement, I'll follow-up.

### comment:10 follow-up:  11 Changed 5 years ago by Ryan J Ollos

Resolution: → fixed assigned → closed

(In [10936]) Fixes #5907: Version 0.4.1: A regression in the 0.4 version of the plugin prevented the helppage link from displaying when helppage.newwindow was False.

The helppage link can now be specified as a resource in any realm, not just a wiki page.

### comment:11 in reply to:  10 ; follow-up:  12 Changed 5 years ago by Steffen Hoffmann

The helppage link can now be specified as a resource in any realm, not just a wiki page.

True. This has actually been started in [10431] by removing code to always add the mandatory 'wiki/' prefix. I suggest to treat this as a regression bug on it's own, because tickets claiming this are foreseeable, right?

Still I like more flexibility and existing code rules over just wild ideas. So consider the following patch for improving the situation.

### Changed 5 years ago by Steffen Hoffmann

some improvements over current, backwards-incompatible state of handling helppage option

### comment:12 in reply to:  11 ; follow-up:  13 Changed 5 years ago by Ryan J Ollos

True. This has actually been started in [10431] by removing code to always add the mandatory 'wiki/' prefix. I suggest to treat this as a regression bug on it's own, because tickets claiming this are foreseeable, right?

Still I like more flexibility and existing code rules over just wild ideas. So consider the following patch for improving the situation.

I'd like to extract the code from your patch into a private method __get_helppage_link to allow for unit testing in the future, and to keep the main filter_steam method concise and readable. This is the programming approach I'd take in general, but given your much stronger knowledge of Python and Trac, I wanted to check with you before making this change.

### comment:13 in reply to:  12 ; follow-up:  14 Changed 5 years ago by Steffen Hoffmann

You're welcome. Glad you like it.

I'd like to extract the code from your patch into a private method __get_helppage_link ... but ... I wanted to check with you before making this change.

Sure, valid move, even more after my proposed extensions. But let me go ahead and attach the current patch as another preliminary development state:

• changed order of processing to exclude any (explicitly or assumed) wiki resource from being checked - rationale: I'd like to be able to create the configured wiki page, if not existent.
• idea salvaged from abompard's patch: possibility to use an anchor (wiki-only) - rationale: Special care must be taken to prevent it from getting urlencoded inside the Href object.
• interpreting whole string as wiki page ID is an intentional fallback, if resource validation fails, because
1. the resource realm is unknown or
2. the resource with the given ID doesn't exist in that realm

Please discuss, if anything is questionable or undesired from your point of view.

I'll create the private method now and attach a third patch for review.

### Changed 5 years ago by Steffen Hoffmann

another preliminary development state with some improvements

### comment:14 in reply to:  13 ; follow-up:  15 Changed 5 years ago by Ryan J Ollos

Please discuss, if anything is questionable or undesired from your point of view.

Only thing that jumps out initially is that the first 2 if statements might be transformed to an if-elif, dropping the if not link check on the second.

### comment:15 in reply to:  14 ; follow-up:  16 Changed 5 years ago by Ryan J Ollos

Only thing that jumps out initially is that the first 2 if statements might be transformed to an if-elif, dropping the if not link check on the second.

That is, unless req.href can return None, but I'm not familiar with a condition in which this can occur.

### comment:16 in reply to:  15 ; follow-up:  18 Changed 5 years ago by Steffen Hoffmann

Only thing that jumps out initially is that the first 2 if statements might be transformed to an if-elif, dropping the if not link check on the second.

That is, unless req.href can return None, but I'm not familiar with a condition in which this can occur.

Me too, but I can't map your comment to current code now. Maybe it's just too late, anyway, let me drop in the rework towards a private method.

Patch has been tested on Trac 0.11 and 0.13dev-r10880, so I noticed another issue: recently added javascript_quote from trac.util.text is not available before Trac 0.11.3 . Still like full backwards-compatibility? Suggestion included in the following patch.

### Changed 5 years ago by Steffen Hoffmann

more improvements: private method _get_helppage_link and backwards-incompatibility down to Trac 0.11

### comment:17 Changed 5 years ago by Steffen Hoffmann

Forgot: Please make sure to include explicit file encoding ('utf-8') as suggested in latest patch, thanks.

### comment:18 in reply to:  16 ; follow-up:  20 Changed 5 years ago by Ryan J Ollos

Patch has been tested on Trac 0.11 and 0.13dev-r10880, so I noticed another issue: recently added javascript_quote from trac.util.text is not available before Trac 0.11.3 . Still like full backwards-compatibility? Suggestion included in the following patch.

Certainly. I'll take the code since your have generously provided it, though I probably wouldn't have gone out of my way to do it myself up until now.

I've been developing on 0.11.7 and testing on 0.12.2, which I had felt gave me sufficient coverage to satisfy most users. I have a dream of setting up automated builds to test installation and perhaps run unit tests against a greater range of versions, but I'm a ways off from that. Until then, I might modify my approach and start developing against 0.11.0, like you are doing, at least for plugins that have been around quite a while such as this one.

### comment:19 follow-up:  21 Changed 5 years ago by Steffen Hoffmann

A note of caution: 0.13dev has great auto-preview like wiki side-by-side editor in tickets, now not only for comments but all fields.

But this preview interferes with plugin JS code, because it uses redirection internally. So I can't see the drop-down list anymore. Seen functional regression in other plugins as well, i.e. TracTicketTemplatePlugin.

Will investigate and most probably open another ticket dedicated to 0.13 compatibility - no need to do that in 0.11 branch I guess.

### comment:20 in reply to:  18 Changed 5 years ago by Steffen Hoffmann

Cc: Michael Renzmann added; Steffen Hoffmann removed

... I have a dream of setting up automated builds to test installation and perhaps run unit tests against a greater range of versions, but I'm a ways off from that.

Me too, once again.

<wild-dream> Super-cool, if this could even become a centralized service a-la ci@trac-hacks.org, to provide independent insight into code quality with a few mouse clicks.</wild-dream>

### comment:21 in reply to:  19 Changed 5 years ago by Ryan J Ollos

Will investigate and most probably open another ticket dedicated to 0.13 compatibility - no need to do that in 0.11 branch I guess.

Yeah, that is definitely appreciated. I'm actually surprised I didn't destroy 0.11 compatibility with the jQuery UI changes made in #8141. After I made those changes, I was thinking this would be the case, and I'd have to rewind some changes on the 0.11 branch and start a trunk version or at least a 0.12 branch. I did break compatibility with the AutocompleteUsersPlugin. I intend to fix this issue in #9599.

### comment:22 Changed 5 years ago by Ryan J Ollos

This never seems to get high enough on my priority list lately for me to test and commit the patch. Steffen, if convenient for you, feel free to push your patch straight to the repository. You have repository-wide commit access now so this is possible, and I trust your work!

### comment:23 follow-up:  25 Changed 5 years ago by Steffen Hoffmann

Owner: changed from Ryan J Ollos to Steffen Hoffmann reopened → new

Ok, will take care, but IIRC you did some change in-between, so I'll have to adapt/test/re-validate.

Splitting the latest path in two should do nicely: Add compat-function, and in another step add the flexible option parser code. Thanks for the thumbs-up.

### comment:24 Changed 5 years ago by Steffen Hoffmann

(In [11201]) KeywordSuggestPlugin: Check helppage config option for valid realm ID, refs #5907.

Try Trac's resource system to create appropriate links to any existing resource. We gain backwards-compatibility by continuing to assume a wiki resource, if options value has no leading /.

### comment:25 in reply to:  23 Changed 5 years ago by Steffen Hoffmann

Status: new → assigned

Ok, will take care, but IIRC you did some change in-between, so I'll have to adapt/test/re-validate.

Splitting the latest path in two should do nicely: Add compat-function, and in another step add the flexible option parser code. Thanks for the thumbs-up.

Done, so we have all that I was speaking about in another comment before. Now it is a very versatile, backwards-compatible option and allows for virtually any possible customization to the 'Keywords' labeled link. Feedback welcome.

### comment:26 Changed 3 months ago by Ryan J Ollos

Status: assigned → new

### comment:27 Changed 3 months ago by Ryan J Ollos

Owner: Steffen Hoffmann deleted

### Modify Ticket

Action
as new The ticket will remain with no owner.