Fix dotted dict processing dots in second+ level keys#2745
Conversation
jwortmann
left a comment
There was a problem hiding this comment.
The new tests don't actually test what was fixed here.
I would expect something like
d = DottedDict({"editor.codeActionsOnSave": {"source.fixAll": "explicit"}})
self.assertIsNone(d.get("editor.codeActionsOnSave.source"))
self.assertIsNone(d.get("editor.codeActionsOnSave.source.fixAll"))Note that the second test shows that we cannot access individual nested keys with a dot in it anymore, but I guess that is fine.
Actually I think this could be solved as well by recursively testing the full key name first, before splitting parts from the right end (I haven't tried a concrete implementation). LSP/plugin/core/collections.py Lines 44 to 51 in dfd3a91 Then if a language server asks for |
|
Interesting point you are bringing up, I didn't think of "get" not working for those. I'm curious how it works in VSCode when server asks. Only if there is an interoperability issues I would consider changing it. |
|
I see the same behavior in VSCode: For a setting like: {
"editor.codeActionsOnSave": {
"source.fixAll.eslint": "always"
},
}here are relevant requests: |
I assume you created this request just for testing, or is there a real server that uses a Here is what I was thinking about in my comment above, how it might work: def get(self, path: str | None = None) -> Any:
"""
Get a value from the dictionary.
:param path: The path, e.g. foo.bar.baz, or None.
:returns: The value stored at the path, or None if it doesn't exist.
Note that this cannot distinguish between None values and
paths that don't exist. If the path is None, returns the
entire dictionary.
"""
return self._d if path is None else self._get(self._d, path)
@staticmethod
def _get(d: dict[str, Any], path: str) -> Any:
segments = path.split('.')
for idx in range(len(segments), 0, -1):
key = '.'.join(segments[:idx])
if key in d:
if rest := '.'.join(segments[idx:]):
val = d[key]
if isinstance(val, dict):
return DottedDict._get(val, rest)
return None
return d[key]
return None |
|
I've tested with a real server based on https://github.com/microsoft/vscode-extension-samples/tree/main/lsp-sample that I've just extended to ask for different sections. I don't see any point in trying to make it work differently than how it works in VSCode. That reminds me that I should verify that any server settings or custom plugin code currently doesn't rely on old behavior... |
|
Found two packages that would be affected. Those will be fixed. |
Co-authored-by: jwortmann <[email protected]>
Make
DottedDictonly process top-level keys as "dotted".Some examples:
=>
{ "a": { "b": { "c": 1 } } }=>
{ "a": { "b": { "c.d": 1 } } }DottedDict.update(other)also works like that -otheronly gets keys expanded at the first level.DottedDict.set(path, other)works as before - the wholepathis expanded(there is a bit of repetition in the code (
_merge()andset()are pretty similar) and not sure about naming ofmergesince we sometimes call it "update".Fixes #2744