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

Language server initialisation adjustments #580

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Language server initialisation adjustments #580

merged 2 commits into from
Feb 26, 2024

Conversation

charlieegan3
Copy link
Member

This PR makes two changes:

  • Error launching LSP server in Neovim with string type initializationOptions #578 raised an issue with the unmarshalling of the initializationOptions field. We don't use this field so it has been dropped.
  • We an issue when there is no root URI set in the initialize request. This PR disables workspace functions such as aggregate violations and linting of unopened files when this is the case (when a single file has been opened).

this is unused and behaves differently in different clients, see #578

Signed-off-by: Charlie Egan <charlie@styra.com>
This disables features that make use of the Root URI, namely
the linting of more than one file, use of config file, and
linting of unopened files.

This is only used when a language server is started with no
client root URI.

Signed-off-by: Charlie Egan <charlie@styra.com>
Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Just one question.

}

// attempt to load the config as it is found on disk
file, err := config.FindConfig(strings.TrimPrefix(l.clientRootURI, "file://"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was file:// not an issue on windows? Or it’s not applicable here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, need to double check this. Will save a TODO for this.

@anderseknert anderseknert merged commit 21a0dc4 into main Feb 26, 2024
3 checks passed
@anderseknert anderseknert deleted the lsp-fixes branch February 26, 2024 23:03
charlieegan3 added a commit to charlieegan3/vscode-opa that referenced this pull request Mar 1, 2024
When VS code starts, there is an issue where the configuration is
'changed' multiple times during start-up. This happened too quickly for
the activatedRegal function to reliably test if regal was running
already and caused many servers to be started.

This meant that sometimes messages would be duplicated since many
servers would send the same message.

The minimum regal version has also been bumped, to contain fixes for
non-workspaced vs code instances. (see StyraInc/regal#580)

Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
charlieegan3 added a commit to charlieegan3/vscode-opa that referenced this pull request Mar 1, 2024
When VS code starts, there is an issue where the configuration is
'changed' multiple times during start-up. This happened too quickly for
the activatedRegal function to reliably test if regal was running
already and caused many servers to be started.

This meant that sometimes messages would be duplicated since many
servers would send the same message.

The minimum regal version has also been bumped, to contain fixes for
non-workspaced vs code instances. (see StyraInc/regal#580)

Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
anderseknert pushed a commit to open-policy-agent/vscode-opa that referenced this pull request Mar 1, 2024
When VS code starts, there is an issue where the configuration is
'changed' multiple times during start-up. This happened too quickly for
the activatedRegal function to reliably test if regal was running
already and caused many servers to be started.

This meant that sometimes messages would be duplicated since many
servers would send the same message.

The minimum regal version has also been bumped, to contain fixes for
non-workspaced vs code instances. (see StyraInc/regal#580)

Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
Co-authored-by: Charlie Egan <charlieegan3@users.noreply.github.com>
srenatus pushed a commit to srenatus/regal that referenced this pull request Oct 1, 2024
* Drop InitializationOption from config

this is unused and behaves differently in different clients, see StyraInc#578

Signed-off-by: Charlie Egan <charlie@styra.com>

* Run LS in workspaceless mode when no root URI set

This disables features that make use of the Root URI, namely on
the linting of more than one file, use of config file, and
linting of unopened files.

This is only used when a language server is started with no
client root URI.

Signed-off-by: Charlie Egan <charlie@styra.com>

---------

Signed-off-by: Charlie Egan <charlie@styra.com>
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 this pull request may close these issues.

2 participants