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

Make analyzable file types customizable #1944

Closed
simolus3 opened this issue Aug 24, 2019 · 9 comments
Closed

Make analyzable file types customizable #1944

simolus3 opened this issue Aug 24, 2019 · 9 comments
Labels
in editor Relates to code editing or language features is enhancement
Milestone

Comments

@simolus3
Copy link
Contributor

A plugin in the analysis server could support analysis results for file types that are neither Dart or HTML. For these files, DartCode wouldn't send analysis requests to the analysis server because the file ending is not matched by isAnalyzable:

const analyzableLanguages = ["dart", "html"];
const analyzableFilenames = [".analysis_options", "analysis_options.yaml", "pubspec.yaml"];
return analyzableLanguages.indexOf(document.languageId) >= 0
|| analyzableFilenames.indexOf(path.basename(fsPath(document.uri))) >= 0;

Is there a reason not all files in the current workspace are marked analyzable? If there is, it would be amazing if we could make the analyzable file endings customizable. Then, plugins that support custom file endings could instruct users to mark the respective file ending as analyzable.

@DanTup
Copy link
Member

DanTup commented Aug 24, 2019

Is there a reason not all files in the current workspace are marked analyzable?

For files that are analyzeable, we send the full contents to the server every time they're opened, and then send every key stroke. For files that the server won't do anything with, this seems really wasteful (there are three processes involved, and each is serialising and deserialising).

I don't think there's a way right now for a plugin to tell the server what it wants to handle - I've somewhat assumed there aren't many plugins and I could just maintain this list (we need to be sure that if we add to it, then people that aren't running the plugin don't get errors from the server that the files shouldn't be sent).

Having a setting to control this seems reasonable - though out of interest, which plugin are you using? If it's something common that others may also be using (eg. not something private you're working on), then it might be worthwhile adding to the list (assuming it doesn't cause issues as described above).

@DanTup DanTup added this to the v3.4.0 milestone Aug 24, 2019
@DanTup DanTup added awaiting info Requires more information from the customer to progress in editor Relates to code editing or language features is enhancement labels Aug 24, 2019
@simolus3
Copy link
Contributor Author

simolus3 commented Aug 24, 2019

Thanks for the quick and detailed response!

I don't think there's a way right now for a plugin to tell the server what it wants to handle

The plugin will submit a list of globs to analyze to the server when replying to the plugin.versionCheck request (docs). I would assume that the analysis server then delegates the work accordingly. The analysis server doesn't inform tooling about these globs though. If I remember correctly this was planned, but then dropped because of complexity.

I'm working on the plugin at the moment, but I intend to make it publicly available. The files have an ending of .moor and contain sql, my plugin would provide syntax highlighting and some basic static analysis.

If a user-controllable list of file endings is feasible, I would prefer moor not to be included by default, mainly because I'm not sure how well the plugin is actually going to work. If you think the limited amount of plugins doesn't justify a configuration option, I could live with moor being added to the list - I assume it's rare to have .moor files in a Dart project if it's not using my library.

@DanTup
Copy link
Member

DanTup commented Aug 26, 2019

Thanks for the info! For now, I think a user-supplied list probably makes the most sense - it's the least risky if something is wrong because the user can easily turn it on/off without needing new releases.

@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label Aug 26, 2019
@DanTup
Copy link
Member

DanTup commented Aug 26, 2019

I've added this new setting, which is an array of file extensions:

"dart.additionalAnalyzerFileExtensions": [
	"moor",
]

The current languages use the language ID, but I wasn't sure if VS Code would assign them to unknown files, so this one uses the extensions (as returned by Node's path.extname with the dot stripped off the front).

@DanTup DanTup closed this as completed in 0d1f914 Aug 26, 2019
@DanTup
Copy link
Member

DanTup commented Aug 26, 2019

There's a new beta build here if you'd like to try it out:

https://github.com/Dart-Code/Dart-Code/releases/tag/v3.4.0-beta.2

Release notes aren't ready yet, but the list of issues here covers what's in it:

https://github.com/Dart-Code/Dart-Code/milestone/97?closed=1

If you try it out, let me know if you hit any issues.

@simolus3
Copy link
Contributor Author

simolus3 commented Aug 26, 2019

Thanks, I just tried it out and I can verify that VS Code indeed sends analysis.updateContent requests for the file endings added. It seems like no subscription for syntax highlighting is sent though (grepping for setSubscriptions from the instrumentation log, id 6 is sent from the IDE, and id 2 is sent from the analysis server to the plugin afterwards):

1566829618526:Req:{"id"::"1","method"::"server.setSubscriptions","params"::{"subscriptions"::["STATUS"]},"clientRequestTime"::1566829617838}
1566829619049:Req:{"id"::"6","method"::"analysis.setSubscriptions","params"::{"subscriptions"::{"CLOSING_LABELS"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"],"FOLDING"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"],"OCCURRENCES"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"],"OUTLINE"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"]}},"clientRequestTime"::1566829618469}
1566829619058:Req:{"id"::"7","method"::"completion.setSubscriptions","params"::{"subscriptions"::["AVAILABLE_SUGGESTION_SETS"]},"clientRequestTime"::1566829618469}
1566829636512:PluginReq:{"id"::"2","method"::"analysis.setSubscriptions","params"::{"subscriptions"::{"OUTLINE"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"],"FOLDING"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"],"OCCURRENCES"::["/home/simon/IdeaProjects/moor/extras/plugin_example/lib/test.moor"]}}}:file::///home/simon/.dartServer/.plugin_manager/58b23ef5b003d93bd30fbec6b91a24a7/analyzer_plugin/bin/plugin.dart::

Does DartCode run their own syntax highlighting independent of the analysis server? I also can't see a HIGHLIGHTS subscription when I'm adding a Dart file to the project.

If it's not too much work, can DartCode use the highlights from the analysis server? I guess this only makes sense for extensions added to "dart.additionalAnalyzerFileExtensions" as it implies that the user doesn't have any other tooling for these files that could interfere.

@DanTup
Copy link
Member

DanTup commented Aug 26, 2019

If it's not too much work, can DartCode use the highlights from the analysis server?

I really wish I could! :) Unfortunately VS Code does not support syntax highlighting from a language server, the highlighting needs to be shipped as a textmate grammar (microsoft/vscode#585 is the request for VS Code support).

I'm forever chasing syntax highlighting issues, so if they ever supported this, I'd definitely adopt it. There has been some discussion on there recently (performance testing other options), but it's not clear if they're going to make it really dynamic or have extensions provide it in another declarative format that performs better.

@simolus3
Copy link
Contributor Author

simolus3 commented Jan 6, 2020

@DanTup it looks like semantic tokens are available in the insiders channel. So I'm wondering if you have plans to integrate that in the Dart extension in the medium term?

I would love to help here and open a PR. I just figured I'd ask first, so that I don't do duplicate work or something that's unlikely to get merged.

@DanTup
Copy link
Member

DanTup commented Jan 7, 2020

@simolus3 it's definitely something I'd like to move to - though I'm not kept up with the new API recently so I'm not currently sure of the effort involved. It might be just a simple mapping - similar to something like the Hover provider), I'm not sure.

It's unlikely I'll get to it in the next few months though, so if you're interested in a PR, I'm certainly happy to review and assist. If so, open an issue for it (we have a few about bugs as a result of not having this, but it probably makes sense to have a specific one for moving to this to keep discussion in one place).

Note: Work is in progress to move to LSP, so any work on existing VS Code provider APIs will likely be removed. If it turns out to be significant effort, we may wish to consider holding off until it's supported in LSP and then just implementing it here.

(Also FWIW we can't merge/ship anything until the API is both finalized and released to stable so any work would have to remain on a branch for now).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features is enhancement
Projects
None yet
Development

No branches or pull requests

2 participants