GNOME Bugzilla – Bug 531085
Improper handling of links (note and URL) terminating with '[', '}', or ')'
Last modified: 2011-06-26 05:26:30 UTC
Tomboy doesn't handle urls terminating with a '[' properly. For example, try to paste a url like this: http://www.ncbi.nlm.nih.gov/sites/entrez?Db=gene&Cmd=DetailsSearch&Term=SLC40A1[Gene%2FProtein+Name] on a tomboy note. The last bracket will be not highlighted as a part of the link. I can paste a screenshot if you think it is necessary. Other information:
Confirmed in 0.11.0.
Also a problem with '}'.
*** Bug 567914 has been marked as a duplicate of this bug. ***
Also a problem with ')'. Also, note names with any of these characters at the end have linking problems. If I have a note called "testing (parens)", and add that text in another note, the link appears. However, if I add a space to that text, the link disappears.
*** Bug 484264 has been marked as a duplicate of this bug. ***
This was fixed for note links in bug #323845. I am hoping that a fix for bug #436994 might fix the rest of this...
So this is not fixed, though there is an unreviewed patch by Florian attached to bug #436994. My feeling is that it's a pretty common use-case to put URLs in square brackets and parentheses. Maybe more common than URLs including those characters (although Wikipedia may tilt the odds back again). Anyway, maybe we shouldn't link too aggressively. What do people think about accepting ], }, and ), but only if the first character of the text being evaluated isn't the corresponding opening character? Would like to see such a patch.
Created attachment 129969 [details] [review] modify regexp rule to match url ending with anything but spaces This is the patch added to bug #436994.
Thanks for the patch, Florian. Any thoughts on comment #7?
Created attachment 130006 [details] [review] identical rewrite regexpr for urlwatcher I just rewrote the regexpr to make it easier to read. There is a "regression test" writing different version of the regexpr to log. I agree with comment #7. Interesting regexp exercice...
Here's probably another link bug: Just type the following in a note /bin/bash AAA /bin/bash (That's two tabs behind AAA.) The underline starts under the whitespace of the second tab.
Created attachment 130041 [details] [review] modify regexp rule to match some url with parenthesis Well, that's all what I could come to. This patch changes the way url are detected. If a ( or [ or { is found at the beginning of the string, then ) and ] and } are forbidden in the address. Else any non space is allowed. (see screenshot)
Created attachment 130042 [details] screen shot for attachment id=130041
(In reply to comment #11) > Here's probably another link bug: Just type the following in a note > > /bin/bash AAA /bin/bash > > (That's two tabs behind AAA.) > > The underline starts under the whitespace of the second tab. > I think you're in the wrong bug. You want bug #436994, which was fixed for the 0.13.6 release (for which I have yet to send a release email because I haven't made the Windows installer yet).
(In reply to comment #14) > I think you're in the wrong bug. You want bug #436994, which was fixed for the > 0.13.6 release Right, and fixed with trunk, thanks :)
(In reply to comment #13) > Created an attachment (id=130042) [edit] > screen shot for attachment id=130041 > I cannot open the screenshot. Bugzilla says the image contains errors?
(In reply to comment #16) > (In reply to comment #13) > > Created an attachment (id=130042) [edit] > > screen shot for attachment id=130041 > > > > I cannot open the screenshot. Bugzilla says the image contains errors? > Worked for me when I downloaded and opened with eog.
That file is no PNG.. file says: PostScript document text conforming at level 3.0
sorry, that the first screenshot I take. I used "import filename" it works well with evince.
Created attachment 130109 [details] [review] modify regexp rule to match some url with matching parenthesis All right, this should work fine. I think this implement comment #7. > accepting ], }, and ), but only if the first character of the text being > evaluated isn't the corresponding opening character? Would like to see such a > patch. note : accepts also any other non-space character. like http://kj;,;: test cases are : (http://toto.com}])etc {http://toto.com)]}etc [http://toto.com)}]etc ( http://toto.com}])etc { http://toto.com)]}etc [ http://toto.com)}]etc http://toto.com}])etc http://toto.com)]}etc http://toto.com)}]etc screenshot follows (hopefully true png ;-) )
Created attachment 130110 [details] screen shot for patch 130109
Created attachment 131432 [details] https url URLs beginning with "https" are no longer correctly handled (I *think* they used to be). Just "http" works fine...
Created attachment 131443 [details] [review] rewrite of previous patch Sorry, I can't reproduce your screenshot, https works for me like http. this may be due to incorrect handling of '+' starting the new code in the patch. I moved the '+' signs to make it clearer.
*** Bug 584999 has been marked as a duplicate of this bug. ***
Trying this patch out and it's working really well, with one big exception: /path/to/somewhere ~/path/in/homedir Those no longer work, sadly. I'm playing with it now, though. These, at least seem to work: [/path/to/somewhere] [~/path/in/homedir] Also, I need to reintegrate my fix from this commit: http://git.gnome.org/cgit/tomboy/commit/?id=ca90435a4112c0870f5bf389bee971b7540b53c7 By the way, Florian, there are some extra changes not related to this bug in your latest patch.
Created attachment 136072 [details] [review] possibly better patch It's kind of messily-formatted right now, but I think this may be better. Wanted to attach before I forgot/lost it (heading out for a bit). Will follow up later! :-)
Comment on attachment 136072 [details] [review] possibly better patch committed bde1d9a547936b40d40fa627acf36f5b3b3dbd01 Sandy's patch handles all the cases above.
Actually, we're matching all strings that are preceded with ( [ { Again, underscore indicates start and end of a link (_thisisnotanotebuthasalink_ [_thisisnotanotebuthasalink_ {_thisisnotanotebuthasalink_
Created attachment 189803 [details] [review] Fixes the regression with regular text starting with [ ( {
Comment on attachment 136072 [details] [review] possibly better patch Revert "Fix handling of ] } ) in links" This reverts commit bde1d9a547936b40d40fa627acf36f5b3b3dbd01. Patch introduces a regression. Since it is related to linking, we'll consider it severe enough that we should revert. There is a fix in bugzilla, once reviewed we can reapply the patches.
Review of attachment 189803 [details] [review]: commit 32bc77abf14c4163f89a00174adcc560f123eec3
committed to master 32bc77abf14c4163f89a00174adcc560f123eec3