Bugzilla – Full Text Bug Listing
| Summary: | Hyperlinking support for Trac should handle disabled (escaped) links. | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Radoslaw Jozwik <rjozwik> | ||||||||||||
| Component: | Mylyn | Assignee: | Steffen Pingel <steffen.pingel> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
| Severity: | enhancement | ||||||||||||||
| Priority: | P3 | CC: | greensopinion | ||||||||||||
| Version: | 3.0 | Keywords: | helpwanted | ||||||||||||
| Target Milestone: | 3.0.2 | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Linux | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Radoslaw Jozwik
We currently don't have the resources to address this bug in the Mylyn team. If there is interest in the community to resolve this bug we would be happy to take a contribution. The relevant code is in TracHyperlinkUtil.findTracHyperlinks(). Created attachment 109954 [details]
mylyn/context/zip
David, this is one of the patterns "[A-Z][a-z0-9]+[A-Z]\\w*". Do you know how to modify it to not match when there is an exclamation mark in the beginning? reassign to me and I'll take a look Created attachment 110097 [details]
a patch that fixes the regex and adds a JUnit
used a negative 0-length look-behind
added a JUnit that tests various combinations of the regex.
Created attachment 110098 [details]
mylyn/context/zip
Awesome patch! Thanks a lot. This is really advanced usage of regular expressions. A test case was when there was not preceding character, e.g. when matching "WikiPage". I changed the expression prefix to this instead: "(?<![!])". Does that make sense? Thanks for the test cases I have merged them with the existing TracHyperlinkUtilTest. If I run your test as a JUnit plug-in test I get an IllegalAccessError: Although TracHyperlinkUtil and TracHyperlinkUtilTest appear to be in the same package OSGi uses separate class loaders for the plug-ins making the fields in TracHyperlinkUtil inaccessible to the test class. Please note that by convention the Mylyn test plug-ins do not follow the package structure of plug-ins under test. (In reply to comment #7) > Awesome patch! Thanks a lot. This is really advanced usage of regular No problem > matching "WikiPage". I changed the expression prefix to this instead: > "(?<![!])". Does that make sense? Looks good to me. I actually meant to do that before, but somehow my brain wasn't working. > Thanks for the test cases I have merged them with the existing > TracHyperlinkUtilTest. If I run your test as a JUnit plug-in test I get an > IllegalAccessError: Although TracHyperlinkUtil and TracHyperlinkUtilTest appear > to be in the same package OSGi uses separate class loaders for the plug-ins > making the fields in TracHyperlinkUtil inaccessible to the test class. Hmm... that's odd. I've used this same-package-different-plug-ins approach for JUnits for a long time with no problems. Is the build producing sealed jar files? > Please note that by convention the Mylyn test plug-ins do not follow the package > structure of plug-ins under test. I understand. How else do you propose to test package-scoped fields/members? Created attachment 110120 [details]
mylyn/context/zip
Created attachment 110121 [details]
new patch that obsoletes the older one
turns out that the old regex didn't work properly if the wiki link was at the start of input.
Also care has to be taken with this look-behind type regex because there are some major bugs with it on Java 5.
The attached patch works on Java 5 and Java 6, and has two extra test cases for testing at the start of input.
This blog post discusses some of the possibilities to test package visible elements: http://michaelscharf.blogspot.com/2008/04/is-osgi-enemy-of-junit-tests_17.html . > Hmm... that's odd. I've used this same-package-different-plug-ins approach for > JUnits for a long time with no problems. Is the build producing sealed jar > files? No, all packages are exported. It works if you run it as a regular JUnit test but not when run as a JUnit Plug-in test (unless the test plug-in is a "friend" of the plug-in that's tested). > > Please note that by convention the Mylyn test plug-ins do not follow the > package > > structure of plug-ins under test. > > I understand. How else do you propose to test package-scoped fields/members? We should bring this up on one of the calls again. So far the policy has been to only test the publicly accessible interface of classes. I have added your test case and renamed it to TracHyperlinkUtilStandaloneTest to indicate that it should not be run as a plug-in test. I'm not sure I understand why the additional group in the regular expression is required. All test cases pass on Java 5 and 6 when I use this as the prefix "(?<!!)" and the additional group breaks the hyperlink detector? (In reply to comment #11) > This blog post discusses some of the possibilities to test package visible > elements: > http://michaelscharf.blogspot.com/2008/04/is-osgi-enemy-of-junit-tests_17.html . > Thanks for the link. The test-plug-in-as-fragment idea looks good, however at first glance it looks like it'll only work if the test plug-in is testing classes from one other plug-in (having one plug-in to test classes from many other plug-ins won't be an option). Using package-scoped members/fields is a great way to test the internals of a class. It would be a shame to lose that capability. > > No, all packages are exported. It works if you run it as a regular JUnit test > but not when run as a JUnit Plug-in test (unless the test plug-in is a "friend" > of the plug-in that's tested). this is a problem for many of the WikiText tests. > > I understand. How else do you propose to test package-scoped fields/members? > > We should bring this up on one of the calls again. So far the policy has been to > only test the publicly accessible interface of classes. There was an exception made to this policy for the WikiText plug-ins since they came with many (over 300) JUnit tests. > > I have added your test case and renamed it to TracHyperlinkUtilStandaloneTest to > indicate that it should not be run as a plug-in test. Sounds good. I hope that this JUnit will still be run as part of the build. > > I'm not sure I understand why the additional group in the regular expression is > required. All test cases pass on Java 5 and 6 when I use this as the prefix > "(?<!!)" and the additional group breaks the hyperlink detector? Now that I'm testing it again it's working for me as well on both Java 6 and Java 5. Must have been some minor change that I made previously that was only working on Java 6. Not sure why the extra group would cause problems, but the main thing is that it's working now. Good work. |