-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(intellij): add dart sdk constraint to language server #4469
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
Conversation
...rc/main/java/com/bloc/intellij_generator_plugin/language_server/BlocLanguageServerFactory.kt
Outdated
Show resolved
Hide resolved
import kotlin.test.assertFalse | ||
import kotlin.test.assertTrue | ||
|
||
class VersionComparatorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding tests! 💯
} | ||
|
||
override fun setEnabled(enabled: Boolean, project: Project) { | ||
// TODO: Implement this with settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you leaving this todo intentionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see on the documentation of LSP4IJ, this setter is for permitting to add an option to programmatically set on and off the LSP. We gonna use it to add an option to disable the LSP in near future but it's outside of this PR scope i think.
Little refactor added |
@@ -63,4 +71,31 @@ class BlocLanguageServerFactory : LanguageServerFactory { | |||
.createNotification(content, type) | |||
.notify(project) | |||
} | |||
|
|||
override fun isEnabled(project: Project): Boolean { | |||
val blocToolsVersion = getBlocToolsVersion() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this method is being called many times and results in a really laggy user experience so we should probably rework this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, i gonna check to clean this tomorrow.
...lugin/src/main/java/com/bloc/intellij_generator_plugin/language_server/BlocLanguageServer.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ✅
Thanks as always for the contributions! 🙏
Status
READY
Breaking Changes
NO
Description
Fix #4468 for IntelliJ
Type of Change