Bugzilla – Full Text Bug Listing
| Summary: | Provide open implementation command (was: Open Declaration should traverse to function declaration where possible) | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | John Arthorne <john.arthorne> |
| Component: | JS Tools | Assignee: | Curtis Windatt <curtis.windatt.public> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | Carolyn_MacLeod, curtis.windatt.public, Michael_Rennie |
| Version: | unspecified | ||
| Target Milestone: | 10.0 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
| Bug Depends on: | 478764 | ||
| Bug Blocks: | 479682, 479684 | ||
Eric, what do you think? It's a good question actually; one one hand the simple (non-recursive) lookup is deterministic and a second use of Open Declaration can be used to get to the actual method impl... on the other hand it's true that in most (if not all) cases it's valid to presume that the user is much more interested in the code than how it surfaced on hi page. I personally want the second but if we do this we completely lose the ability to track where things are actually coming from... To get you started I provided all the tern plugin bits, the handler, the new server query type, the command impl and the NLS'ing. All thats really left is implementing the guts of the open impl tern plugin. For the guts of the plugin, you have a handle to the running server, so you can very easily call findDecl, findType, etc to find the impl from the original declaration. For example (simple type export case): 1. call findDecl 2. use its location to find the enclosing node 3. if an AssignmentStatement, grab the right hand side 4. if the RHS is an Identifier, find its decl I dark launched the Open Implementation command as part of: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=6bcc31150ff4912b8e2062302fe9db12c93cba37 I pushed some new API from Tern for you to directly use #findDef: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=55fc4eb0e71069cb8c40eb9ca9e222ba99688581 Here are some sample tests: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=247940aa943d6a4bbb340540798986c9b828ffbc There are some that test in the same file (the impl is in the same file that we activate the command in) and some that test cross file impls. I commented a few of the tests that are passing, but show incorrect location marking. There is one test that fails (but shouldn't). http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=4d865fceb9f9f3f2972dbca1b5e8428eafe6a793 Added additional test for constructor set on prototype with exported object. Need to figure out which of these cases we expect to work: 1) Foo(){} Foo.prototype.constructor=Foo; return {Foo:Foo} (this is the new test and works 100% now) 2) Foo(){} Foo.prototype.constructor=Foo; return Foo; (content assist works, but open implementation doesn't) 3) Foo(){} return Foo; (content assist fails to find the constructor, open impl fails too) (In reply to Curtis Windatt from comment #7) > 2) Foo(){} Foo.prototype.constructor=Foo; return Foo; > (content assist works, but open implementation doesn't) > 3) Foo(){} return Foo; > (content assist fails to find the constructor, open impl fails too) For both of these cases, if you run the findDef on the NewExpression node (new a()) rather than just the identifier (a) it returns the correct origin file. Another case that I found not working: 1. grab the ace editor from its repo (https://github.com/ajaxorg/ace) 2. once cloned, navigate to the ace.js file 3. place the caret in the last EditSession part on the line: var EditSession = require("./edit_session").EditSession; 4. run open impl I would expect it to find the var decl in edit_session.js Open decl will find the assignment in edit_session.js, looks like we are just missing the final hop to the variable declaration for EditSession http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=2dd0f39133094ab578533fa99996e95656574602 Rewrites the open implementation command. All the tests, new and old are passing. I have removed the darklaunch flag. I will open new bugs for Mike's case from ACE (we have to handle member expressions in two different cases) as well as for Node exports tests which I wrote a crossfile test for but it times out. |
If you have a function assigned to a variable, "open declaration (F3)" will jump to the variable declaration, rather than the function declaration. I think jumping to the function declaration where possible would be more useful. The common case is with a require statement: var Project = require('../core/Project'); ... project = new Pro<>ject(config, logger); I have used <> to indicate current caret position. If I press F3 at that caret location, it takes me to the "var Project" line, but what I really want to see is the function declaration in the file ../core/Project.js.