Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Jump-to-definition jumps to top of file before jumping to definition of exported function #4296

Open
njx opened this issue Jun 20, 2013 · 10 comments
Assignees

Comments

@njx
Copy link
Member

njx commented Jun 20, 2013

  1. Open brackets.js
  2. Find a call to Dialogs.showModalDialog()
  3. Put the cursor in it and choose Navigate > Jump to Definition

Result: the first time you do this, you'll see Dialogs.js open scrolled to the top of the file, and then after a short pause it will jump to the actual definition.

@njx
Copy link
Member Author

njx commented Jun 20, 2013

Per the comments on #3863, this is because we need a CodeMirror instance in order to find the actual definition of the exported function, so either we would need to create one separately before opening the file, or we'd need some other way to tokenize the file before opening it.

@ghost ghost assigned eztierney Jun 20, 2013
@njx
Copy link
Member Author

njx commented Jun 20, 2013

Low priority to @eztierney

@eztierney
Copy link
Contributor

We could tokenize the file ahead of time with acorn, but that seems like a waste of work since we will then be tokenizing it twice when we actually open the file.

@njx
Copy link
Member Author

njx commented Jun 20, 2013

Maybe there's a way we could just prevent the editor from showing itself before we're ready to tell it what position to scroll to. Would need a little thought as to the best way to do this.

@njx
Copy link
Member Author

njx commented Oct 18, 2013

Bumping to medium priority. This feels really bad--there's often a long pause between the time it switches to the new file and the time it jumps to the correct location, and it's actively confusing.

@ghost ghost assigned dangoor Nov 16, 2013
@njx
Copy link
Member Author

njx commented Nov 16, 2013

To @dangoor since it's related to code hints (though the solution might actually be more in the timing of editor switches). Nominating for sprint 35.

@dangoor
Copy link
Contributor

dangoor commented Dec 5, 2013

Not going to get to this in 35. Moving to 36.

@peterflynn
Copy link
Member

Just calling Document._ensureMasterEditor() (in some clean manner) should ensure that there's an offscreen CodeMirror instance to get tokens from, without forcing it to become visible yet. That usage pattern is already well tested for other cases like making edits in an inline editor.

To avoid redundant tokenization you just need to make sure that the editor is made visible before any other Documents are created (so the offscreen, "un-anchored" editor isn't cleaned up by DocumentManager._gcDocuments()). Or you could add the file to the working set so it's "anchored" but I don't think Jump to Definition does that today...

@dangoor dangoor added this to the Brackets 1.0 milestone Mar 17, 2014
@peterflynn
Copy link
Member

Reviewed - here's our suggested plan:

  • First briefly investigate why it's so slow -- can this just be made faster?
  • If fixing that seems expensive, then do a simple fix where we throw up a wait cursor and leave the current editor visible until ready to jump directly to the match. (We wouldn't want to just delay showing the new editor, because then it looks like nothing happened and the user might move onto another task before getting pulled into the new file unexpectedly a moment later).

@peterflynn peterflynn removed this from the Brackets 1.0 milestone Aug 15, 2014
@peterflynn
Copy link
Member

Reviewed -- removing 1.0 milestone. Although it's ugly, it does still work reliably... and a fix may be somewhat tricky (and ideally we'd spend some effort just trying to make it faster, too).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants