Jump to definition goes to exports line instead of function definition

[njx]

1. In the Brackets source, open brackets.js
2. Find a method call to a module, e.g. Dialogs.showModalDialog(...)
3. On the method name, Jump to Definition

Result: Goes to the exports line where that method is assigned, instead of going to the actual definition of the function.

[njx]

@eztierney - I thought this was working at some point, but I might never have actually tried it. It looks like @jeffkenton has a solution for this in #3847 - basically, use Tern to figure out which file, then use our JSUtils.findMatchingFunction() in that file to search for the actual definition (which just does a simple regexp-based search). But I wonder if there's a way to actually trace back to the original definition using Tern?

[eztierney]

@njx - I think I saw it work sporadically too, but I think that may have been a bug in tern that got fixed - I recall seeing a bunch of fixes around position info.

I don't know if theres a way to force tern to automatically do this, but we could manually do it if tern doesn't support it. When we jump-to-def/quick-edit if the result gives us an expression like

exports.foo = foo

then we could just ask tern for the definition of the right hand side, and we could keep doing that till we got to something that looked like a function.

This would handle the case of

exports.foo = bar;

as well, which the current searching won't handle. I don't know how common that is, but I have seen a few cases like

exports._myPrivateFunc = myPrivateFunc; // no underscore here defeats search for '_myPrivateFunc'

[eztierney]

So it looks like marijn recently added a flag to do exactly what we want, at least for functions.

When we do a 'definition' query to tern we can set the 'typeOnly' flag to true, and it looks like the definition query will trace back to the original function. The only problem is that 'typeOnly' means the 'definition' query fails when the definition isn't a function/object literal. But maybe we can work around that - we could do the 'typeOnly' query first, and fall back to the normal definition logic if it didn't work.

Need to do more experimenting, but this looks like it might do exactly what we want.

[jeffkenton]

That's what Pete and I saw with typeOnly. It works for functions but not for var definitions.

[njx]

It seems like we should push on getting this to do what we want eventually in Tern. If that can't happen for some reason, then maybe we should do the same thing JS Quick Edit is doing (basically falling back to the JSUtils regexp-based search).

(BTW, when you say it doesn't work for vars, do you mean it doesn't work for functions assigned to a var, or do you mean it doesn't work for non-function vars? If the latter, that's probably okay for now since we can just make jump to definition only work for functions. But if the former, then we should definitely see if we can get that fixed.)

[jeffkenton]

It doesn't work for non-function vars. It works fine for functions assigned to vars.

[njx]

Ah. Then it seems like it would be worth switching to that for functions at least.

[jeffkenton]

Explain? It's working for functions assigned to vars.

[njx]

Maybe I'm misunderstanding the thread :) Currently, jumping to definition for a function assigned through "exports" is not actually going to the definition. It sounds like setting the typeOnly flag would fix this, and it works for both function declarations with names and functions assigned to vars, so my suggestion was that we actually go ahead and use it for functions.

[eztierney]

Yes, It seems like tern does what we want, even if it isn't the most elegant way of doing it.

I have some code that implements the fallback scheme I described above, and it seems to work (for functions, vars that are functions, and for vars that aren't functions). I still need to fixup some issues around what gets selected/where we put the cursor, but that shouldn't be too hard.

[pthiess]

Reviewed

[njx]

Nominating for sprint 27. I'm running into this a lot.