Allow passing LS implementation specific options. #524
Conversation
Made use of in csharp and intelephense LS
|
@johngambleubind took a bit longer, but here it is: the possibility to configure the download path through adjusting project settings. If some setting is missing, you can build on top of this and add it in a PR |
We discussed that it should be in |
| initialize_params["initializationOptions"] = {"licenceKey": license_key} | ||
| initialization_options["licenceKey"] = license_key | ||
|
|
||
| custom_intelephense_settings = self._solidlsp_settings.ls_specifics.get(self.__class__.__name__, {}) |
There was a problem hiding this comment.
We originally discussed that we would use the enum item's value that is associated with the language server.
Do you think the class name is a better option?
Right now, the Language enum is actually a LanguageServer enum - and perhaps we should rename it accordingly - and use it accordingly.
There was a problem hiding this comment.
I think the enum value would be preferable, because the user is already familiar with the respective values.
And we should probably document the possible options in serena_config.yml to make this accessible.
There was a problem hiding this comment.
The documentation in serena_config.template..yml is not updated to describe the new usage and does not list the possible options. Still says this:
Maps the language server class name to the options. Have a look at the docstring of the constructors of the LS implementations ...
…ation Use this as key for ls_specifics. Also moved ls_specifics to SerenaConfig instead of ProjectConfig
# Conflicts: # src/serena/config/serena_config.py # src/solidlsp/ls.py
|
@opcode81 Since I did not want to repeat the code connecting the enum to the class, I have "simplified" the |
MischaPanch
force-pushed
the
feature/solidlsp_ls_specific_config
branch
from
306a9e4 to
3f3fc0a
Compare
| @@ -266,6 +267,7 @@ def create_language_server( | |||
| log_level: int = logging.INFO, | |||
| ls_timeout: float | None = DEFAULT_TOOL_TIMEOUT - 5, | |||
| trace_lsp_communication: bool = False, | |||
| ls_specifics: dict[Language, Any] | None = None, | |||
There was a problem hiding this comment.
Shall we just add SerenaConfig as an argument? What do you think?
(I am not sure what I prefer)
| @@ -33,6 +33,12 @@ trace_lsp_communication: False | |||
| # whether to trace the communication between Serena and the language servers. | |||
| # This is useful for debugging language server issues. | |||
|
|
|||
| ls_specifics: {} | |||
There was a problem hiding this comment.
Do you like the name "ls_specifics"? I think it's a bit vague.
How about "ls_specific_settings" or "ls_advanced_settings"?
Agreed. Not super nice, but good enough. |
Made use of in csharp and intelephense LS
@opcode81 : it would probably have been better to configure ls_specifics in serena_config instead of the project config, but that would have required a refactoring and I didn't want to do that (not important enough IMO)