Bug 288427 - url hyperlinks with parenthesis are not detected correctly
Summary: url hyperlinks with parenthesis are not detected correctly
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: 3.2   Edit
Hardware: Macintosh Mac OS X - Carbon (unsup.)
: P2 minor (vote)
Target Milestone: 3.4   Edit
Assignee: Abner Ballardo CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, contributed, helpwanted
Depends on:
Blocks:
 
Reported: 2009-09-03 00:40 EDT by Ketan Padegaonkar CLA
Modified: 2010-02-25 23:39 EST (History)
2 users (show)

See Also:

Attachments
mylyn/context/zip (939 bytes, application/octet-stream)
2009-09-03 02:01 EDT, Steffen Pingel CLA 		no flags Details
Patch (3.50 KB, patch)
2010-02-21 13:19 EST, Abner Ballardo CLA 		no flags Details | Diff
mylyn/context/zip (1.30 KB, application/octet-stream)
2010-02-21 13:19 EST, Abner Ballardo CLA 		no flags Details
Patch with fix and tests (5.99 KB, patch)
2010-02-22 10:53 EST, Abner Ballardo CLA 		steffen.pingel: iplog+
Details | Diff
mylyn/context/zip (1.81 KB, application/octet-stream)
2010-02-22 10:53 EST, Abner Ballardo CLA 		no flags Details
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ketan Padegaonkar CLA 2009-09-03 00:40:47 EDT 
I've tried this with bugzilla, I'm not sure if this is the case with all other editors (or an issue with the HyperLinkDetector) that mylyn uses.

Mylyn does not seem to detect hyperlinks when they contain a parenthesis within them: for e.g. http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/Logger.html#trace(java.lang.Object). You'll notice that the closing ')' is not part of the hyperlink either in the textbox where you type comments or in the discussion/comments section where it is shown in read-only mode.
Comment 1 Steffen Pingel CLA 2009-09-03 02:01:04 EDT 
This is intentional to handle the more common case where the parenthesis is not part of the hyperlink (e.g. http://elcipse.org). I can see how that breaks your use case though. It might work to count the number of open parenthesis in the hyperlink and include as many trailing closing parenthesis. 

I have attached a context in case anyone wants to pick up this bug.
Comment 2 Steffen Pingel CLA 2009-09-03 02:01:20 EDT 
Created attachment 146347 [details]
mylyn/context/zip
Comment 3 Abner Ballardo CLA 2010-02-21 12:46:00 EST 
I want to contribute fixing this bug.
Comment 4 Abner Ballardo CLA 2010-02-21 13:19:38 EST 
Created attachment 159716 [details]
Patch

Proposed fix for bug #288427
Comment 5 Abner Ballardo CLA 2010-02-21 13:19:41 EST 
Created attachment 159717 [details]
mylyn/context/zip
Comment 6 Steffen Pingel CLA 2010-02-21 15:11:15 EST 
Thanks for the patch Abner. It would be great if you could add a couple of test cases to TaskUrlHyperlinkDetectorTest and attach a new patch. I'll review and apply it then.
Comment 7 Abner Ballardo CLA 2010-02-22 10:53:46 EST 
Created attachment 159800 [details]
Patch with fix and tests

Hi Steffen, this new patch has the bug fix and the tests added to TaskUrlHyperlinkDetectorTest.
Comment 8 Abner Ballardo CLA 2010-02-22 10:53:52 EST 
Created attachment 159801 [details]
mylyn/context/zip
Comment 9 Steffen Pingel CLA 2010-02-25 23:10:45 EST 
Great patch! I have applied with a minor simplification: checkForExistingTasklist() now returns false if the user cancels instead of storing the response in a field.
Comment 10 Steffen Pingel CLA 2010-02-25 23:39:57 EST 
That comment was intended for a different bug but mostly applies anyways. Thanks for the patch Abner. I have applied it without modifications.