Bugzilla – Full Text Bug Listing
| Summary: | Odd marking of occurrences | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Michael Rennie <Michael_Rennie> | ||||
| Component: | JS Tools | Assignee: | Michael Rennie <Michael_Rennie> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | curtis.windatt.public, mamacdon | ||||
| Version: | 5.0 | Flags: | curtis.windatt.public:
review+
|
||||
| Target Milestone: | 5.0 RC2 | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Created attachment 239837 [details]
fix
The patch fixes up the quirks and shifts the occurrence support to use the token stream. Mark, Curtis, give the patch a spin, it includes some more tests, so make sure to run those as well.
Tests pass and the changes look good, two cases I'm not 100% sure on
1) If you have a large selection, and the start of the selection is in an identifier, mark occurrences runs (this is not new or specific to this fix).
2) Changed the last lines of TextView.js to:
mEventTarget.EventTarget.addMixin(TextView.prototype);
return {TextView: TextView
If you put the carat in or beside the last TextView, the word is not highlighted, but previous occurrences are. This fix is still an improvement over not marking any occurences, but if possible we should find the occurrence where the carat is.
(In reply to Curtis Windatt from comment #2) > 2) Changed the last lines of TextView.js to: > mEventTarget.EventTarget.addMixin(TextView.prototype); > return {TextView: TextView In this case the returnStatement doesn't have an argument at all and no further nodes are entered. Isn't anything we can do about recognizing the occurrence. +1 for RC2 |
When we moved from finding a word to using the AST node at the given offset, we introduced a bit a weirdness. Consider the following snippet: var foo = 1; function f1() {}; function f2() { var bar = new f1(foo); } function f3(foo, bar, baz) {}; If you place the carat after 'f1()' all occurrences of 'f1' are marked, same if you place the carat after 'foo,' (in the f3 decl). The reason is that the AST node contains the immediate + proceeding punctuation ranges in its start + end (Ok I guess, but feels weird). We could fix this by using the findToken API instead of findNode. In theory using the token would give us a performance boost as well as more precise ranges, since we won't walk the entire AST top-down to find the token (like we have to for the node).