GNOME Bugzilla – Bug 436994
file hyperlinks include the space character before them
Last modified: 2009-05-09 13:06:11 UTC
Please describe the problem: When typing a file name, it is highlighted along with the space that precedes the file name (if it's not the start of a line). Using svn revision 1163. Steps to reproduce: 1. Create a new note 2. Type in "foo ~/bar" Actual results: The text " ~/bar" is hyperlinked Expected results: Only "~/bar" should be hyperlinked Does this happen every time? Yes Other information: The leading space in the URL is a known problem: http://svn.gnome.org/viewcvs/tomboy/trunk/Tomboy/Watchers.cs?annotate=HEAD#l380
Setting the default assignee and QA Contact to "tomboy-maint@gnome.bugs".
Thanks for reporting this and sorry for our late response. This is the current behaviour as I understand it: The regular expression in the NoteUrlWatcher class (Watchers.cs) matches preceding whitespace for files. Not only the space character is shown as a link, also a preceding new line (see bug #529568). When the link is activated, the whitespace is removed and the url is opened.
Created attachment 110279 [details] [review] Removed whitespace matching from regex I tried to fix the regular expression and removed the parts that were matching whitespace. I am not very good with regular expressions, so it is possible that I destroyed something. I also do not know what the whitespace matching with (^|\s) was intended to do.
Created attachment 129851 [details] [review] Remove word boundary check and optional / ending in regexpr. Sorry, I don't see your patch working for http://bugzilla.gnome.org/thing) I would do this : see patch. The question is : Do we allow url ending with ) , ( , or even " such as http://bugzilla.gnome.org/thing) http://bugzilla.gnome.org/thing" http://bugzillagnomeorgthing,+:-)@"^ The patch allows everything.
(In reply to comment #4) > Sorry, I don't see your patch working for > http://bugzilla.gnome.org/thing) That's because this bug and the patch is about the space that is hyperlinked before file links. I think there is another bug somewhere for parentheses. I do not know if my patch still works, will test it today.
The patch still works for me, the blank and the new line are not linked any more. Though I am not completely sure what I did back then. Florian, can you attach your patch to bug #531085?
*** Bug 529568 has been marked as a duplicate of this bug. ***
Stefan, committed your patch in r2397. Will look at Florian's as part of bug #531085 as suggested. I'm not sure I understand what corner case was supposed to be covered by the original regex. I hope it is an obsolete one. :-) We really need unit tests for this stuff.
Well, I found out the corner case this was regex was addressing. Try typing "this/that/other", and notice how "/that/other" becomes linked as a URL. Working on a better fix now (probably the same as the previous regex, but without *including* the leading whitespace in the match).
Created attachment 133294 [details] [review] Fix extraneous URL matchings, like this/that/other matching "/that/other", following up on bug #436994. Restore the original regular expression with checks for leading whitespace, but make sure to use zero-width positive lookbehind assertion to keep leading whitespace out of the match.
By the way, I just used git-bz to create comment #10 (and the attachment) in one easy command. http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/ After committing the change locally, the command I used was: git-bz attach bugzilla.gnome.org:436994 HEAD^ Anyway, if somebody wants to play with this before I push it, that would be cool. Release on Monday afaik (they were talking about pushing it back because of delays caused by git migration).
Oh, forgot to mark this is fixed.