refactor: utilize latest lsp_utils #33
Conversation
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
|
Can you wait for new lsp_utils version? I'm planning to introduce common API for that. |
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Since we are waiting for the next lsp_utils release anyway... Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
jfcherng
changed the title
Signed-off-by: Jack Cherng <jfcherng@gmail.com>
|
|
||
| if settings.get("dev_environment") == "sublime_text": | ||
| server_settings = settings.get("settings", {}) # type: Dict[str, Any] | ||
| if self.get_plugin_setting("dev_environment") == "sublime_text": |
There was a problem hiding this comment.
This might sound like a crazy idea but this get_plugin_setting wouldn't be necessary had we put our dev_environment inside the settings object.
The server will ignore unknown properties anyway and we'll get the benefit of merged settings automatically (not that important in this case but could be in other cases).
Notice that it's already happening (in VSCode at least) as the python settings are not really pyright specific in VSCode and the server still receives all of them.
There was a problem hiding this comment.
Not against that but probably adding an extra prefix like sublimelsp.dev_environment?
There was a problem hiding this comment.
I don't feel it's really necessary. We should treat those settings as ours as that's how normal users see them. Users don't need to know the distinction between our and original pyright settings.
IMO.
There was a problem hiding this comment.
having a dedicated prefix makes it more structured imho. now we only have one settings though.
There was a problem hiding this comment.
I'm fine with a prefix but not sure what it would be.
There was a problem hiding this comment.
I propose using the package name as the prefix, i.e., LSP-pyright.dev_environment.
There was a problem hiding this comment.
Sorry but it sounds very redundant to put a LSP-pyright.* key inside the LSP-pyright.sublime-settings or inside the LSP-pyright object in project settings, for example.
I think it only achieves peace of mind for you, and possibly other developers working on the plugin, because you want to keep the settings of pyright and our additions separate. But as I said, I don't believe those should be treated as separate. To normal users, there is no distinction really.
That setting is a bit special as it affects/modifies other settings though. So from that perspective, I guess it might make sense to single it out somehow. But I really can't think of anything better right now. Personally, I would think that a name like environment_type would be enough.
There was a problem hiding this comment.
That setting is a bit special as it affects/modifies other settings though. So from that perspective, I guess it might make sense to single it out somehow.
Does it imply only dev_environment (whatever its name) will be in server settings but not other non-server settings in the future?
There was a problem hiding this comment.
I don't think that implies anything as we don't have strict rules for this. I'm just trying to think of what's best to do here.
But in the end, it's a trivial thing and we should probably not spend too much time discussing it.
Maybe just keep as it is now, even if it complicates the code internally...
There was a problem hiding this comment.
Yeah. I guess that can be discussed later if wanted. https://github.com/neoclide/coc-python#options seems to just use python as the prefix.
resolves #30