GNOME Bugzilla – Bug 584362
Link isn't created.
Last modified: 2011-07-27 12:56:27 UTC
Please describe the problem: I originally sent this bug to Launchpad: https://bugs.launchpad.net/ubuntu/+source/tomboy/+bug/361370 When I create a note with a given name, and then write that name inside other note, a link is created. But, if I write the word "the " before the name, the link is NOT created. I'm attaching a screenshot with an example: Steps to reproduce: 1-I had a note called "point of view". 2-Typing "point of view" in another note creates a link. 3-But if I write "the point of view", the link disappears. 4-In short, writing "the " (with 1 space) before the name of the note makes the link disappear. 5-This doesn't happen if, for example, you put two spaces between "the" and "point of view" This seems to happen randomly with many of my notes, while others work correctly even if I write "the " before the name of the note. Actual results: There is no link. Expected results: There should be a link. Does this happen every time? This happens with some notes, but with them it happens every time. Other information:
Created attachment 135665 [details] screenshot
I can't seem to reproduce this bug, tested with 2 different notes, including one titled "point of view" similar to the reporter. See attached screenshot. I see from the bug report in Launchpad you're using: DistroRelease: Ubuntu 9.04 NonfreeKernelModules: nvidia Package: tomboy 0.14.0-0ubuntu1 I'm using 0.14.1 (on Foresight). (Maybe fixed in 0.14.1?)
Created attachment 135679 [details] Tomboy screenshot
Paul, I have to stress that it seems to happen randomly and I don't really know if it depends on the note's name. Should I attach my .tomboy directory so that you can check if it happens to you?
Created attachment 135683 [details] my .tomboy directory
Thanks David, I will try to look into this with your .tomboy as soon as I can. If anybody else has time to try it out, that would rock!
I have found out that typing "a" before the link has the same effect as typing "the": that is, the link not appearing. Could someone please review this bug? I use tomboy as my main study tool, and this problem is resulting very very impeding.
Created attachment 144109 [details] video showing the problem in action Well David, you aren't completely alone on this one. I'm using Tomboy 1.0.0, and I've uploaded a video showing the problem as it happens to me. In the first sentence of my test note, typing "a weird dream" doesn't link to a preexisting note named "Weird Dream". Typing "weird dream" later in the note - without "a" before it - correctly links to the note. Frustratingly enough, of all of the notes that I tested, "weird dream" was the only one that this happened with. All of the other notes linked correctly in the test note, regardless of whether or not they were preceded with "the" or "a". Also worth noting is 19 seconds into the video. The words "weird dream" flash blue and underlined for a quick second, but then go back to unlinked. Tomboy seems to know, at least for a second, that this should be a link. I'll change the version to 1.0.0, but I won't mark this as NEW until someone can find a way to reliably reproduce this. I'll keep looking at it.
I'm setting the target milestone to Future, as .15.x is in the past. Actually I'm also going to set this as NEW, as some Terminal output confirms that something weird is going on here. after typing "... about a weird dream that i..." I get the following output: [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. [DEBUG]: DoHighlight: 'a weird dream' links to non-existing note. It adds another of these lines every time I add a character. Clearly it knows that this should be a link, but it includes the article, making it impossible to find the note. Why this seems to happen completely at random is still a mystery to me.
Thanks for the helpful clues, Michael. Retargeting for 1.2.
Created attachment 190651 [details] Notes directory Guys, great news. I have discovered what is causing the bug. I'm attaching my notes directory. As it is, the bug happens (e.g. type "marca" and it will link you to a note, but type "the marca" and it won't). However, if you rename the note "The Matter of Britain" to "Matter of Britain", it seems to fix the bug. Therefore, it's my guess that there is something wrong in that one note, that is causing the bug.
Just updating with my current status here: The problem: The problem lies within our TrieTree class. You can easily see the problem with these lines: var trie = new TrieTree (false); trie.AddKeyword ("La p", "Foo"); trie.AddKeyword ("pe", "Bar"); foreach (var m in trie.FindMatches ("La pen")) { Console.WriteLine ("Found key '{0}' at {1} to {2}, with value '{3}'", m.Key, m.Start, m.End, m.Value); } You'll notice that I've the values (think: Note titles) 'La p' and 'pe' (there are lots of ways to get to this bug, but this was the reduced problem from the notes attached). The output will be 'La p' 'La pe' though. The latter is crap and leads to a debug message about a wrong link trying to be created. It's the reason why the original poster won't see a link in his notes: Tomboy doesn't recognize the note title correctly. The cause: The results from the Trie are actually good. It matches correctly, but it tries to track the start/end index of a match in the string and this fails (and I haven't found a good way to do that so far). How to proceed: Actually I'm not sure. The TrieTree class is - obscure and seems to be a line for line port. I don't want to change a lot there, although that might make sense for the future (at least documenting some things, and removing obsolete code - like the index tracking maybe). The real culprit is in Watchers.cs:731-734: if (Manager.Find(hit.Key) == null) { Logger.Debug ("DoHighlight: '{0}' links to non-existing note." , hit.Key); return; } Note that directly afterwards, if we didn't 'return' early because of the wrong hit.Key, we're using the right object. So I think removing the 4 lines above should be the immediate/interim solution: Note hit_note = (Note) hit.Value; Can someone verify my explanation above?
Followup: The solution isn't that easy, unfortunately. The case that we saw here (no links to some notes) would be solved, but because of the wrong indices we'd highlight the wrong part of the input string. I still don't understand why the guard with the early exit is there, but we need to dive into the index tracking in the trie nevertheless.
Okay, I'm now running Tomboy without these problems. Everything seems to work and I cannot reproduce the link problems anymore. The downside? I couldn't follow the TrieTree.cs at all, so I grabbed a paper on Aho-Corasick and rewrote it. The change is limited to that single file. With a lot more comments than the original version I'm at ~140 lines vs. ~260. Because this is a central part of Tomboy (THE central part?) I'm a little nervous now. I'll give it a spin for a short while and clean up the patch (damn VS always fights the formatting here). Some more documentation would be good as well. Afterwards I'd like to post two patches: 1) A new TrieTree.cs implementation, fixing this bug 2) A patch to the NoteWatcher class, removing the (weird?) guards in there
Created attachment 191249 [details] [review] Proposed first patch First shot. This works for me (tm) in limited test cases so far and fixes the bug. I haven't played with lots of notes though and I have several things that I plan to do: * Add automated test cases * Documentation (to avoid ending up in a similar situation again) * Remove unnecessary guards. Nevertheless: This patch should work. Feedback would be greatly appreciated.
Created attachment 191260 [details] [review] Updates the first proposal: Fixes tabs vs. spaces, improves performance (a lot) Fixed spaces instead of tabs and moved the failure graph computation out of the insert keyword method. On my (old) laptop, with the dataset of 900+ notes from the data attached to this bug, this reduces the startup time by a large margin (2 seconds+).
Review of attachment 191260 [details] [review]: Looks good, I think the existing unit tests are still valid and should be included. You can add additional tests later. But if they pass the existing tests, let's commit this.
Review of attachment 191260 [details] [review]: Did you just say IMPROVES STARTUP PERFORMANCE BY 2.5 SECONDS?!?!?!?! ::: Tomboy/Trie.cs @@ +53,3 @@ + public void AddKeyword (string keyword, object pattern_id) + { + if (!case_sensitive) keyword = keyword.ToLower (); bad style, we want: if { statement } @@ +56,3 @@ + + var currentState = root; + for (int i = 0; i < keyword.Length; i++) All of the for/if/while statements in this file need the opening brace on the same line, not the next line. @@ +115,3 @@ + static TrieState FindStateTransition (TrieState state, char value) + { + if (state.Transitions == null) return null; bad style @@ +125,3 @@ + public IList<TrieHit> FindMatches (string haystack) + { + if (!case_sensitive) haystack = haystack.ToLower (); bad style
Comment on attachment 191260 [details] [review] Updates the first proposal: Fixes tabs vs. spaces, improves performance (a lot) Fixed style bugs and committed the bug. I hope to add some automated tests ASAP, but I'd like to use the current development schedule to expose this change to some more users (.. testers). Works for me. Fixes the bug for me.
Should be fixed with the commit explained above and is part of the latest development release as of this writing (1.7.2, from the 25th of July). If you feel that you still experience this problem, please feel free to reopen. Thanks a lot for the very detailed reports and videos.