search

swyddfa / esbonio

A language server for working with Sphinx projects.
https://docs.esbon.io/
135 stars 21 forks source link

Macros like workspaceFolder should be supported #304

Closed lextm closed 2 years ago

lextm commented 2 years ago

I can see that in commit f6bfd3430eb056b457a2abaef53279db0f07b96b you tried to add support of workspaceRoot. However, that's not a desired change in 2022.

The workspaceRoot macro was very old (before VSCode supports multi folder workspaces), and instead workspaceFolder is recommended (and you can find tons of other extensions relying on this new macro like this).

Therefore, to read settings from VSCode you should be ready to handle both workspaceRoot and workspaceFolder (and when writing settings back, you should only use workspaceFolder, but esbonio right now does not write settings).

Such macros should be fully supported by key settings such as buildDir, srcDir, and confDir, so that users can share their VSCode settings with others (on different machines with different setup).

alcarney commented 2 years ago

Multi root support is not something I've thought about that much, since it's not obvious to me how it should work.

Taking esbonio.server.pythonPath as an example if I set it to ${workspaceFolder}/bin/python how does the extension determine which workspace folder to use? Or is it the case that the extension should spin up a separate language server instance for each folder expecting to find a Python executable ${workspaceFolder}/bin/python?

To implement ${workspaceFolder} "properly" for settings such as srcDir, confDir etc, that probably means the server would have to manage a separate Sphinx instance for each folder - something I don't think is trivial in a single Python process due to Sphinx and docutils relying on a certain amount of global state.

lextm commented 2 years ago

For this issue alone (and in the initial phase of esbonio), macro support is useful and can be added to the settings I mentioned (buildDir, srcDir, and confDir). There is no need to include pythonPath if you feel uncomfortable with that, but you should consider the fact that many developers check in .vscode/settings.json to repos and share with others.

A language server can have no support of multi root workspaces, and you just need to auto disable it in the extension when such a workspace is opened. Note that snooty does not support multi root workspaces either. As multi root workspace is a huge topic, you might create a new issue to track investigation on multi root workspace support.

alcarney commented 2 years ago

Are you aware that the server already supports some macros? confDir, srcDir and buildDir support ${workspaceRoot} (and it would be easy enough to add ${workspaceFolder} as an alias to the same logic).

srcDir and buildDir additionally support ${confDir} which allows for settings paths relative to the configured confDir.

Are there any other macros that you feel would be worth implementing?

lextm commented 2 years ago

Thanks. From Git blame I can see the language server supports ${workspaceRoot} months ago. Nice.

The only macro I can think of is ${workspaceFolder} at this moment. For vscode-restructuredtext users, that one alone seems to be sufficient and no request on others in the past few years.

beutlich commented 2 years ago

I see that ${workspaceRoot} works as expected, however when restarting VS Code, the relative paths of "esbonio.sphinx.buildDir" and "esbonio.sphinx.confDir" are replaced by absolute paths (preventing a check-in source control). Do you have any clue?

lextm commented 2 years ago

@beutlich You are talking about https://github.com/swyddfa/esbonio/issues/322 which is going to be resolved once a new version of esbonio is shipped to PyPI.

beutlich commented 2 years ago

Thanks for pointing to the correct (closed) issue. Hope to see it released soon as otherwise the restructuredtext extension > 1.170.0 is not usable for us. Thanks again!