Allow setting root ClientConfig settings

[rchl]

I've realized that the approach using __getattr__ fails horribly when trying to set the value (which I would like to do in some cases). Adding __setattr__ is not really feasible or at least it would make things a lot more complicated...

Just expose all_settings directly.

I'm not convinced about the name. Perhaps should be root_settings or something else?

[jwortmann]

Plugin-specific settings are not known or used by LSP, so I wonder what is the intention to make them writeable? Shouldn't they be read-only?

[rchl]

What you mean?

Settings like command are writable so any other custom ones should also be. In my case I have introduced server_path in some plugins which user can override manually but in some cases it makes sense to set those dynamically also.

[jwortmann]

For example I would expect your server_path to be useful only within on_pre_start_async. Why would you want to mutate that value on the ClientConfig? I would see accessing the plugin-specific custom settings from ClientConfig only as a convenience functionality, but not for storing additional things as a mutatable variable. If you need that later in another method (where?), you could use an instance variable for the plugin class instead (e.g. set in on_initialized_async).

command is writeable, because it is used by LSP itself.

[rchl]

I'm not saying it can't be done differently but at the moment I have a case where in on_pre_start_async, I potentially modify it first and then pass the whole context to lsp_utils which has different logic depending on its value.

[rchl]

So with my use case, the value is not modified for the sake of LSP but another "downstream" code. I don't feel that being able to modify custom root properties is inherently wrong.

That said, I'm not sure I like the all_settings name so I'm open to suggestions there.

[rchl]

So regardless of what we do, I'll keep existing code for now for backward compatibility. Don't want to break anything.

But I've added a "normal" root_settings getter which I think is a better solution overall.

[jwortmann]

This could be typed a bit more strictly as dict[str, Any], right?

[jwortmann]

I guess by returning a reference to the original dict, which is mutable of course, this root_settings property would still make it possible to mutate the settings values on the ClientConfig object itself, despite being a @property...

[rchl]

I guess by returning a reference to the original dict, which is mutable of course, this root_settings property would still make it possible to mutate the settings values on the ClientConfig object itself, despite being a @property...

I do want it mutable though.