Fix #86: Allow configuration of workspace by aykborstelmann · Pull Request #96 · NixOS/nix-idea

aykborstelmann · 2025-12-30T12:18:01Z

If done those changes rather for myself, because I disliked the escaping needed for the --config option of nixd.
I hope those changes can also benefit the lsp support for other people directly and should allow more comfort.
I am aware, that in #68 you discussed switching to lsp4ij, but for the time being this might help people.

Migrate Lsp Settings to Project Settings, since the workspace configuration is project specific and the editor text field requires a project

JojOatXGME

Hi. Firstly, thanks for your PR. 👍

I added some comments. Note that only the comments prefixed with issue: or question: strictly require your input. I am mostly concerned about moving NixLspSettings to the project level (see comment).

I am aware, that in #68 you discussed switching to lsp4ij.

I am happy to see improvements in the existing integration. Switching to lsp4ij has no priority for me after JetBrains made their API available free of charge. While I would provide support when someone wants to work on switching, I don't plan to work on it personally.

JojOatXGME · 2025-12-31T22:12:32Z

        @JvmStatic
-        fun getInstance(): NixLspSettings {
-            return ApplicationManager.getApplication().getService(NixLspSettings::class.java)
+        fun getInstance(project: Project): NixLspSettings {


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?

I guess it also means the language server will be disabled for all users after the update

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.

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.

Yes I also thought about this. However when considering jetbrains own plugins, for example vue.js, those are also project wide configured.
However they come with the default "on" and can only be project wide be turned off. In our case I am not comfortable with defining a default, as this would also include choosing a default language server command.
Lastly we might also mix project wide and application wide, I have done this for another intellij plugin as well. That works by having two services, an application wide service that store in our case the command and if its enabled and an project wide service that would store the configuration.

What do you think ?

I am not comfortable with defining a default

Same for me.

an application wide service that store in our case the command and if its enabled and an project wide service that would store the configuration.

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.

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

Yes. I believe you would customize the workspace configuration per project. I.e. in my case I define the nixd options, so that the current nixos autocompletion is using my flake's options and not those of some channel. Similarly I have an option for home-manager that would also only apply to this project. Even though I would say its likely that someone uses one language server for one project and the other for the other, we might as well apply this to everything.

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.

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 :)

JojOatXGME · 2025-12-31T22:25:53Z

    NixLspServerDescriptor(@NotNull Project project, NixLspSettings settings) {
        super(project, "Nix");
-        myCommand = settings.getCommand();
+        this.settings = settings;


though: Wondering if this could theoretically run into a race condition when the language server is started while the configuration is changed. Note that settings is a shared object. I assume it is safe as long as createCommandLine() and getWorkspaceConfiguration(ConfigurationItem) are called in the same action¹ as this constructor.

I guess ideally, we would make a deep copy of the settings. However, that would make the code more complicated and I guess the worst that can happen is that we temporarily start the old command line with the new settings, just before the language server is restarted anyway. So, I think we can ignore this.

Footnotes

Read and write actions are a concept of IDEA's threading model to ensure isolation of independent operations. ↩

Addressed in b7c49f7

That is probably cleaner in any case.

Addressed

Not sure how your changes addressed any potential race condition. You are still using the shared settings object, you just no-longer store it in a field. I guess your changes at least make it more obvious that there could be a race condition. Anyway, as mentioned before, I would be fine with ignoring the race condition, if it exists. It would not really break the functionality, just maybe cause an unexpected error messages in some very rare edge cases.

Yes, I am sorry, I didn't explain myself.
I thought about it in two different ways.

I somehow was not sure how intellij handles those service (in my head it might be that the behave like hibernate proxies and can be detached / outdated, but probably this is not the case ...). As I don't know that, I figured it would be best to get the current settings via the project.

I had a look at how jetbrains handles this in its own plugins. For the astro plugin, the getWorkspaceConfiguration also uses the shared settings to get the configuration, see https://github.com/JetBrains/intellij-plugins/blob/master/Astro/src/org/jetbrains/astro/service/AstroLspServerSupportProvider.kt

JojOatXGME · 2025-12-31T22:29:32Z

+                val editor = super.createEditor()
+                editor.setHorizontalScrollbarVisible(true)
+                editor.setVerticalScrollbarVisible(true)
+                editor.settings.isUseSoftWraps = true


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

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

Yea, I agree that we don't want a horizontal scrollbar on the settings panel.

JojOatXGME · 2026-01-01T00:25:19Z

+            row {
+                label("Workspace configuration:").resizableColumn()
+            }
+            row {
+                cell(createJsonEditorTextField()).align(Align.FILL)
+                    .bind(


suggestion: Isn't resizableColumn() intended for the text field, and not for the label? It should also be possible to set the label on the cell, but I haven't tested it.

Suggested change

row {

label("Workspace configuration:").resizableColumn()

}

row {

cell(createJsonEditorTextField()).align(Align.FILL)

.bind(

row {

cell(createJsonEditorTextField())

.label("Workspace configuration:", LabelPosition.TOP)

.align(Align.FILL)

.resizableColumn()

.bind(

Yes, good catch, that's much neater. Done in 17520c6

Thanks. I noticed you removed resizableColumn() altogether. Just wanted to mention it in the case it was by accident.

Yes this was intentional, I don't notice an effect in that case. I rather added it to be safe. Sorry for the confusion, I will revert it.

JojOatXGME · 2026-01-01T00:28:57Z

+                return editor
+            }
        }
+        editorTextField.preferredSize = Dimension(0, 150)


question (non-blocking): Has preferredSize here any effect? I expect that the size of the window is decided before this component is loaded.

Yes apparently it has. I am not too familiar with the rendering lifecycle of intellij components, but without it, the editor is simply two lines (in the default of {\n}).
I thought that looks weird, so I added the prefferedSize. What do you think ?

Seems strange to me that setting prefferedSize has this effect. But when it works, it works. 😄 Anyway, looks like you have actually removed the line in commit 17520c6. I assume you have made it work somehow else? (I haven't tried to run the UI code myself yet.)

No, good catch, I accidentally committed my experiments with it. I will remove it in the next round.

JojOatXGME

Nothing new, just informing GitHub that I had already looked at your latest changes.

Feel free to add your opinion regarding my suggestion for where to store the settings. I think that is the only topic which is currently blocking the merge.

Anyway, no pressure. Feel free to take your time. 😅

JojOatXGME · 2026-01-04T07:46:01Z

Sorry for not approving the CI earlier. Looks like the PluginVerifier has a problem with <depends>com.intellij.modules.json</depends> only being declared within nix-idea-ultimate.xml

Plugin 'com.intellij.modules.json' not declared as a plugin dependency for class com.intellij.json.JsonFileType. JSON support has been extracted to a separate plugin.

I guess the easiest solution is to move the dependency declaration to plugin.xml unconditionally.

Fix NixOS#86: Allow configuration of workspace

15d16b3

Migrate Lsp Settings to Project Settings, since the workspace configuration is project specific and the editor text field requires a project

aykborstelmann force-pushed the lsp-workspace-configuration branch from 9f0b622 to 15d16b3 Compare December 30, 2025 13:39

JojOatXGME requested changes Jan 1, 2026

View reviewed changes

aykborstelmann added 4 commits January 1, 2026 14:37

fixup! Add trailing new line

4b3522b

fixup! Use label in dsl syntax

17520c6

fixup! Reduce constructor complexity

b7c49f7

fixup! remove scrollbar visible definitions

1562d0f

JojOatXGME requested changes Jan 4, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

aykborstelmann commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JojOatXGME left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JojOatXGME Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

aykborstelmann Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JojOatXGME Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JojOatXGME left a comment

Choose a reason for hiding this comment

Uh oh!

JojOatXGME commented Jan 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aykborstelmann commented Dec 30, 2025 •

edited

Loading

JojOatXGME Jan 1, 2026 •

edited

Loading

aykborstelmann Jan 1, 2026 •

edited

Loading

JojOatXGME Jan 1, 2026 •

edited

Loading