-
-
Notifications
You must be signed in to change notification settings - Fork 30
Fix #86: Allow configuration of workspace #96
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
base: master
Are you sure you want to change the base?
Changes from all commits
15d16b3
4b3522b
17520c6
b7c49f7
1562d0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| IU-2025.3.1 | ||
| IU-2025.1.2 | ||
| IU-2024.3.6 | ||
| IU-2024.2.6 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,35 +1,41 @@ | ||
| package org.nixos.idea.lsp | ||
|
|
||
| import com.intellij.openapi.application.ApplicationManager | ||
| import com.intellij.openapi.components.BaseState | ||
| import com.intellij.openapi.components.RoamingType | ||
| import com.intellij.openapi.components.Service | ||
| import com.intellij.openapi.components.SimplePersistentStateComponent | ||
| import com.intellij.openapi.components.State | ||
| import com.intellij.openapi.components.Storage | ||
| import com.intellij.openapi.project.Project | ||
| import org.nixos.idea.settings.NixStoragePaths | ||
| import org.nixos.idea.settings.SimplePersistentStateComponentHelper.delegate | ||
| import java.util.ArrayDeque | ||
| import java.util.Collections | ||
| import java.util.Deque | ||
|
|
||
| @Service(Service.Level.PROJECT) | ||
| @State(name = "NixLspSettings", storages = [Storage(value = NixStoragePaths.TOOLS, roamingType = RoamingType.LOCAL)]) | ||
| class NixLspSettings : SimplePersistentStateComponent<NixLspSettings.State>(State()) { | ||
|
|
||
| class State : BaseState() { | ||
| var enabled by property(false) | ||
| var command by string() | ||
| var history: Deque<String> by property(ArrayDeque(), { it.isEmpty() }) | ||
| var configuration by string(DEFAULT_CONFIGURATION) | ||
| } | ||
|
|
||
| var isEnabled: Boolean by delegate(State::enabled) | ||
| var command: String by delegate(State::command, State::history) | ||
| val commandHistory: Collection<String> | ||
| get() = Collections.unmodifiableCollection(state.history) | ||
| var configuration: String? by delegate(State::configuration) | ||
|
|
||
| companion object { | ||
| private const val DEFAULT_CONFIGURATION: String = "{}" | ||
|
|
||
| @JvmStatic | ||
| fun getInstance(): NixLspSettings { | ||
| return ApplicationManager.getApplication().getService(NixLspSettings::class.java) | ||
| fun getInstance(project: Project): NixLspSettings { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: I am a bit worried about moving the settings to the project level. This change would mean that users have to enable the language server for every project separately. I guess it also means the language server will be disabled for all users after the update. Is there any rational you want to share about your decision?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is something we could handle with an migration, by keeping the old application wide settings service and initializing the project wide settings service with it. However I thought the effort is not wort it.
Yes I also thought about this. However when considering jetbrains own plugins, for example What do you think ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Same for me.
I think I would prefer an option where the user would choose which level to use. Similar to how the code style settings allow you to choose profiles either on a project or application level. But without the profiles. Just a checkbox or something. You haven't explained why you want to have project-specific configurations, but I assume you have multiple projects using different setups or styles. In the same way, I could imagine that some project also uses a different language server. What do you think? I could also try to make this work if you prefer to not spend more time on it. Otherwise, I think I would also be fine with just moving the option to the project-level, but we would definitely have to document it in the changelog.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes. I believe you would customize the workspace configuration per project. I.e. in my case I define the
I can imagine your proposal to be convenient, I will give it a try and see how it feels. I still believe we could also only make the workspace configuration a project setting because this is probably the only one that someone would change on a per project level. I will work on this, let's see when I will find the time :) |
||
| return project.getService(NixLspSettings::class.java) | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,37 @@ | ||
| package org.nixos.idea.lsp | ||
|
|
||
| import com.intellij.json.JsonFileType | ||
| import com.intellij.openapi.editor.ex.EditorEx | ||
| import com.intellij.openapi.options.BoundSearchableConfigurable | ||
| import com.intellij.openapi.options.Configurable | ||
| import com.intellij.openapi.project.ProjectManager | ||
| import com.intellij.openapi.project.Project | ||
| import com.intellij.platform.lsp.api.LspServerManager | ||
| import com.intellij.ui.EditorTextField | ||
| import com.intellij.ui.RawCommandLineEditor | ||
| import com.intellij.ui.components.JBCheckBox | ||
| import com.intellij.ui.dsl.builder.Align | ||
| import com.intellij.ui.dsl.builder.AlignX | ||
| import com.intellij.ui.dsl.builder.Cell | ||
| import com.intellij.ui.dsl.builder.LabelPosition | ||
| import com.intellij.ui.dsl.builder.bindSelected | ||
| import com.intellij.ui.dsl.builder.panel | ||
| import com.intellij.ui.dsl.builder.selected | ||
| import com.intellij.ui.dsl.builder.toMutableProperty | ||
| import org.nixos.idea.settings.ui.CommandSuggestionsPopup | ||
| import org.nixos.idea.settings.ui.UiDslExtensions.bindText | ||
| import org.nixos.idea.settings.ui.UiDslExtensions.placeholderText | ||
| import org.nixos.idea.settings.ui.UiDslExtensions.suggestionsPopup | ||
| import org.nixos.idea.settings.ui.UiDslExtensions.validateOnReset | ||
| import org.nixos.idea.settings.ui.UiDslExtensions.validateWhenTextChanged | ||
| import org.nixos.idea.settings.ui.UiDslExtensions.warnOnInput | ||
| import java.awt.Dimension | ||
|
|
||
| class NixLspSettingsConfigurable : | ||
| class NixLspSettingsConfigurable(val project: Project) : | ||
| BoundSearchableConfigurable("Language Server (LSP)", "org.nixos.idea.lsp.NixLspSettingsConfigurable"), | ||
| Configurable.Beta { | ||
|
|
||
| override fun createPanel() = panel { | ||
| val settings = NixLspSettings.getInstance() | ||
| val settings = NixLspSettings.getInstance(project) | ||
| lateinit var enabledCheckBox: Cell<JBCheckBox> | ||
| row { | ||
| enabledCheckBox = checkBox("Enable language server") | ||
|
|
@@ -43,16 +50,34 @@ class NixLspSettingsConfigurable : | |
| it.text.isNullOrBlank() | ||
| } | ||
| } | ||
| row { | ||
| cell(createJsonEditorTextField()) | ||
| .align(Align.FILL) | ||
| .label("Workspace configuration:", LabelPosition.TOP) | ||
| .bind( | ||
| EditorTextField::getText, EditorTextField::setText, | ||
| settings::configuration.toMutableProperty() | ||
| ) | ||
| }.resizableRow() | ||
| }.enabledIf(enabledCheckBox.selected) | ||
| } | ||
|
|
||
| @Suppress("UnstableApiUsage") | ||
| override fun apply() { | ||
| super.apply() | ||
| reset() // Update UI components to use normalized property values | ||
| for (project in ProjectManager.getInstance().openProjects) { | ||
| LspServerManager.getInstance(project).stopAndRestartIfNeeded(NixLspServerSupportProvider::class.java) | ||
| LspServerManager.getInstance(project).stopAndRestartIfNeeded(NixLspServerSupportProvider::class.java) | ||
| } | ||
|
|
||
| private fun NixLspSettingsConfigurable.createJsonEditorTextField(): EditorTextField { | ||
| val editorTextField = object : EditorTextField(null, project, JsonFileType.INSTANCE, false, false) { | ||
| override fun createEditor(): EditorEx { | ||
| val editor = super.createEditor() | ||
| editor.settings.isUseSoftWraps = true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Not sure if I personally would enable soft-wraps here. What was the rationale for enabling it? Would it make sense to use the user's code style settings? Also are the scrollbars not visible by default? O.o
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I just chose them because I thought it was sensible. Maybe I would even reconsider it and use softwrap without any scrollbars. If we don't use softwrap the editor takes as much horizontal space as it needs which leads to a horizontal scrollbar of the settings panel, which I would say should not happen (the scrollbar settings don't have any effect as far as I can tell so I would remove them). So I would also not let the user choose.
I remove the scrollbars in 1562d0f
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, I agree that we don't want a horizontal scrollbar on the settings panel. |
||
| return editor | ||
| } | ||
| } | ||
| return editorTextField | ||
| } | ||
| } | ||
|
|
||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.