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

Think about Splitting Compile From Language Feature Step #27

Closed
tricktron opened this issue Jul 22, 2022 · 0 comments · Fixed by #33
Closed

Think about Splitting Compile From Language Feature Step #27

tricktron opened this issue Jul 22, 2022 · 0 comments · Fixed by #33

Comments

@tricktron
Copy link
Owner

tricktron commented Jul 22, 2022

Most Language Features need information from a correctly compiled file. As a result, I designed the two language features to always compile the file first: e.g DiagnosticLSP.compileAndGetDiagnosticsLSP.

This leads to the following dependency chain: languageFeatureService->languageFeatureLSP->languageFeature->compilerHelper.

Consider splitting this again in two steps: First compile and then do language feature work.
This leads to the following two dependency chain:
languageFeatureService -> compilerHelper
languageFeatureService -> languageFeatureLSP -> languageFeature

Right now, I don't know which way is better to go forward. I keep this issue to remind me again to consider this architectural change/possibility when it might be needed.

Remember: Combining is always easier than splitting.

Edit: After some thinking:
Who should decide when to compile? Right now the language feature decides which doesn't make any sense: E.g every hover leads to a compile call, which does not hurt because the frege compiler has a good caching implemented.

Compilation should be driven by code changes. That means that the change event should call the frege compiler and the language feature only gets what it needs from the global.

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.

1 participant