JS hints for Document are missing many methods

[peterflynn]

1. Open Document.js
2. Add a new line at the end of the Document constructor
3. Type this. and observe hints

Result: Contains some methods (addRef(), _makeEditable()) but missing many others (getText(), refreshText(), replaceRange(), getLine(), etc.).

It seems like all functions below _makeEditable() -- i.e. starting with _makeNonEditable() -- are missing. But I can't see anything odd about the syntax there that would trip things up...

[njx]

I'm finding that JS code hinting seems to be missing a lot of stuff, even things that are within the same file (and thus shouldn't be running into limitations related to project size). I'm having a hard time reproducing them outside of the Brackets project though.

In particular, I have a file where I have the exact same issue--this. isn't hinting any methods from the class I'm in.

Nominating for sprint 31. Assigning to @dangoor since I think he's the new point person for JS code hints.

[dangoor]

I think our first step should be updating Tern. We haven't done that for a while. This sounds potentially like a bug.

[dangoor]

We have updated Tern and Acorn and I can still reproduce this. I just tried pasting Document.js into the Tern demo page and it is able to properly find things like getText(), so this may be a bug on our end.

[njx]

Reassigning for me to look at for now.

[dangoor]

I looked at this on the plane yesterday. At one point, I saw that Tern had all of the properties for Document.prototype, but a bunch went away before completions were returned. There are a few places in Tern where properties are deleted or filtered but I hadn't found the culprit before my battery ran out.

[dangoor]

I'm going to reclaim this and move it to Sprint 32, since we're past the pencils down day for Sprint 31.

[dangoor]

The problem here is the logic at: https://github.com/adobe/brackets/blame/master/src/extensions/default/JavaScriptCodeHints/ScopeManager.js#L590

According to the commit message for cb8a0e9, the rationale for this is:

Implement requests with partial file text to reduce memory usage for large files.

"Large" is considered 250 lines

The Tern demo handles this differently, just moving "large file" (250 lines, again) updates into a setTimeout and not updating the text at all when there's a completion request. One difference, though, between what we do and what the Tern demo does is that we display the hints immediately. The Tern demo displays them only on request, likely giving Tern enough time to process the file.

The fix here is going to require some consideration, because the original change was considered to be part of the solution to #3977. I'm going to take a quick look at the memory performance of the current Tern demo with large files (ember.js was cited as one such large file).

One simple solution here may just be to increase what we consider to be a "large file" to be something more reasonable (2,000 lines?)

[dangoor]

I put Ember.js (debug build) into the test_dep.js tab in the Tern demo. 36,471 lines. The code hinting there still works fine. Chrome reports memory usage at 143MB resident, which is up from 61MB before dropping Ember in.

That's not completely terrible. Of course, depending on how we're handling things, I can imagine memory leaking very quickly at that rate (we send the file across for each completion request, which is different from the Tern demo's operation).

[njx]

Ah. That would explain why code hints sometimes feel like they just completely fail to work. 250 lines isn't very much. (I've always been concerned that we seem to have put in a number of limits like this without documenting them anywhere, so it's hard to evaluate in what cases code hints are and aren't supposed to work.)

When you say we send the file across for each completion request, I'm assuming that's true just for the current file (to make sure it's up to date)?

[dangoor]

Yes, just the current file is sent across. In fact, the request to Tern is basically "give me completions for this file I'm giving you right now at position Y" as opposed to "give me completions for file X which you already have at position Y" which is what the Tern demo does.

[njx]

Ah. But doesn't the Tern demo also need to deal with changes made in the current file since it was last sent over?

Anyway, it does seem like we might want to rethink how we're supplying the files at some point.

[dangoor]

The Tern demo deals with changes via the CodeMirror change events. So the updating of the file happens asynchronously with the completions, at least from what I could see. I'm not certain that exact process would work for us, because the Tern demo requires the user to manually invoke completions whereas we automatically run completions as the user types. There may not be a suitable delay for making sure Tern knows the proper context for the completion.

[dangoor]

Moving to Sprint 34 to avoid introducing risk into sprint 33.

[jasonsanjose]

FBNC @peterflynn

[peterflynn]

Confirmed fixed