Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move completions to new compiler testing infrastructure #59

Closed
olafurpg opened this issue Nov 25, 2017 · 7 comments · Fixed by #64
Closed

Move completions to new compiler testing infrastructure #59

olafurpg opened this issue Nov 25, 2017 · 7 comments · Fixed by #64
Assignees

Comments

@olafurpg
Copy link
Member

In #51 and #55 we moved signatureHelp and hover to a new testing infrastructure. This new infrastructure makes it very productive to polish the output from the presentation compiler using ~metaserver/testOnly -- tests.compiler. This workflow is way more productive than doing publishLocal, open vscode, manually type code and see what it produces.

The completion provider can also benefit from some polishing in how the completions are ordered, for example that fields of a class take prioritity over defaults like toString/finalize

@olafurpg
Copy link
Member Author

PS. I've started working on this.

@olafurpg olafurpg self-assigned this Nov 25, 2017
@olafurpg
Copy link
Member Author

Actually, would someone be interested in working on this? I have a WIP diff here https://github.com/scalameta/language-server/compare/master...olafurpg:completions?expand=1 that can be used as a base. There are other issues I can work on instead, for example building semanticdbs on the fly with the presentation compiler.

@olafurpg olafurpg removed their assignment Nov 25, 2017
@gabro
Copy link
Member

gabro commented Nov 26, 2017

I can work on this. I've already started to hack around with completion priorities a few days ago, so I have a vague idea of how to tackle this.

@olafurpg
Copy link
Member Author

Awesome! Just realised why completions started crashing a lot now, I removed the _CURSOR_ trick in the signatureHelp PR. Adding _CURSOR_ avoids many parse errors and enables completions on parts like User.<COMPLETE>

fe14433#diff-be76cbaabee1e6378859da7ffe659ab1L60

We should probably add _CURSOR_ in Compiler.addCompilationUnit, even if that's used for more than completions.

I suspect with this change we can avoid this hacky catch here

https://github.com/scalameta/language-server/blob/b160931177b6d9d0d351ec364cfbca70669fba08/metaserver/src/main/scala/scala/meta/languageserver/compiler/CompilerUtils.scala#L15

@gabro
Copy link
Member

gabro commented Nov 26, 2017

I'm not sure I understand the _CURSOR_ trick, but is it just to make sure the parser doesn't fail when completing at the end of a line?

We should probably add _CURSOR_ in Compiler.addCompilationUnit, even if that's used for more than completions.

addCompilationUnit as no offset in input, where should we add the __CURSOR__?

@gabro
Copy link
Member

gabro commented Nov 26, 2017

Ah, nvm, got it. I think your intuition is right, I've added it back and it seems to be working. I'll push it to a branch soon.

@gabro
Copy link
Member

gabro commented Nov 26, 2017

Scratch that, it seems to have no effect :(

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

Successfully merging a pull request may close this issue.

2 participants