Skip to content
This repository has been archived by the owner on Oct 9, 2024. It is now read-only.

Messages originated from ASL should use language-server-protocol: notificationMessage #15

Closed
ssbarnea opened this issue Sep 9, 2021 · 4 comments
Labels
bug Something isn't working enhancement New feature or request portability Related to ability to use the language server with multiple editors and on multiple platforms.

Comments

@ssbarnea
Copy link
Member

ssbarnea commented Sep 9, 2021

After investigation made with @webknjaz yesterday, we concluded that use of showInformationMessage() or showErrorMessage() inside the language server is wrong as this would prevent use of the language-server on other editors than vscode.

Apparently language-server-protocol has an api that can be used for this use case: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#notificationMessage

The idea is that the ASL should produce the notification and is up to the extension to process it or not. Is not mandatory for each client to implement ability to process these notifications from the language server and that is ok.

One example of processing notifications can be see at https://github.com/redhat-developer/vscode-java/blob/master/src/standardLanguageClient.ts#L152-L181

It worth mentioning that this info was provided by our friends from #devtools-vscode channel from coreos slack instance.

Anyone is welcomed to work on addressing this, just add a comment.

@ssbarnea ssbarnea added bug Something isn't working portability Related to ability to use the language server with multiple editors and on multiple platforms. labels Sep 9, 2021
@ssbarnea ssbarnea changed the title Messages originated from should use language-server-protocol: notificationMessage Messages originated from ASL should use language-server-protocol: notificationMessage Sep 9, 2021
@webknjaz webknjaz added enhancement New feature or request new Use track issue requiring triage labels Sep 9, 2021
priyamsahoo referenced this issue in priyamsahoo/ansible-language-server Sep 15, 2021
* Fix special characters in workspace and file paths (#15)

* Advise yamllint installation and use (#16)

* Fix handling of files with CRLF line endings (#20)

* Mimic linter's config file search algorithm (#22)

Since ansible-lint is executed from the root directory of the workspace
the algorithm that it uses to find configuration for a particular file
is not involved. To compensate, the extension now mimics that behavior,
searching the directory structure, going up from the investigated file.

At the same time, it is still possible to force a particular config file
through arguments provided in the extension settings.

The configuration file that is actually used is now correctly identified
for the purpose of marking configured finding groups as warnings.
ekmixon pushed a commit to ekmixon/ansible-language-server that referenced this issue Oct 15, 2021
* Fix special characters in workspace and file paths (ansible#15)

* Advise yamllint installation and use (ansible#16)

* Fix handling of files with CRLF line endings (ansible#20)

* Mimic linter's config file search algorithm (ansible#22)

Since ansible-lint is executed from the root directory of the workspace
the algorithm that it uses to find configuration for a particular file
is not involved. To compensate, the extension now mimics that behavior,
searching the directory structure, going up from the investigated file.

At the same time, it is still possible to force a particular config file
through arguments provided in the extension settings.

The configuration file that is actually used is now correctly identified
for the purpose of marking configured finding groups as warnings.
@priyamsahoo
Copy link
Contributor

@yaegassy, does this issue affect your dev for the vim extension ?

@webknjaz webknjaz removed the new Use track issue requiring triage label Nov 8, 2021
@yaegassy
Copy link
Contributor

yaegassy commented Nov 8, 2021

@priyamsahoo Hmm, I don't know if this change will affect the Vim extension.

Vim has a variety of LSP client plugins (extension), such as coc.nvim, vim-lsp, and Neovim built-in LSP. Since coc.nvim is a VSCode-like LSP client, client-side adjustments may be relatively easy to handle.

vim-lsp and Neovim built-in LSP and other LSP clients basically only allow you to start the Langauge server and configure the settings related to the startup (configuration options, initializationOptions, etc.). In fact, you could add the ability to handle notifications on the client side, but it may not be easy.

If the addition of the ability for each client to handle notifications is not mandatory, but optional, and it is not a core feature of ASL, then of course it would not be a problem.

@priyamsahoo
Copy link
Contributor

Ah alright. Thanks for the feedback @yaegassy :)

@ssbarnea
Copy link
Member Author

I this is already implemented now, that i how we send info about enviorment.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working enhancement New feature or request portability Related to ability to use the language server with multiple editors and on multiple platforms.
Projects
None yet
Development

No branches or pull requests

4 participants