Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there any rationale preventing from implementing an editor-agnostic config file feature? #133

Closed
jsamr opened this issue Dec 10, 2021 · 5 comments
Labels
1-question ❓ Issue type: General or specific question about LTeX 2-upstream Issue status: Bug is caused by some dependency, might have to wait before continuing 3-not-our-issue Issue resolution: This issue cannot be solved in LTeX, but in some dependency

Comments

@jsamr
Copy link

jsamr commented Dec 10, 2021

That is, the possibility to configure LTEX with a simple JSON or YML file.

Could be implemented with the help of https://github.com/davidtheclark/cosmiconfig

Would be very convenient for open source projects :-)

@jsamr jsamr added the 1-question ❓ Issue type: General or specific question about LTeX label Dec 10, 2021
@jsamr jsamr changed the title Is there any rationale preventing from implementing an editor-agnostic settings file feature? Is there any rationale preventing from implementing an editor-agnostic config file feature? Dec 10, 2021
@valentjn
Copy link
Owner

Did you read the docs of LTEX CLI, in particular the option --client-configuration?

@jsamr
Copy link
Author

jsamr commented Dec 10, 2021

I didn't catch this one, but you have to explicitly interact with the language server to enable this behavior. Is there any way to use a .ltexrc.json file at the root of a workspace and have the editor extension pass it on to the server? I haven't seen such cases documented (at least for the VSCode client). The motivations for such feature:

  1. Editor-independent
  2. Avoid committing local, context-dependent configurations to an SCM (such as .vscode/settings.json)

@valentjn
Copy link
Owner

valentjn commented Dec 11, 2021

Ah, I see. No, there is no such way.

The reason is that the Language Server Protocol (LSP) puts the responsibility for the configuration to the language client. Servers should request and fetch the config from the client via workspace/configuration. LTEX LS wants to adhere to the philosophy of the LSP, and we shouldn't work a way around it.

In addition, it would be even more confusing that it already is to find out where a particular setting comes from, considering that editor settings, magic comments, Babel commands, external setting files, etc. all can influence the settings as well.

Regarding point 1: The LSP is already editor-agnostic with its workspace/configuration request, but the storage of the config is not, as configuration management is the responsibility of the client. If you want to change that, then it's an issue of the LSP spec.

Regarding point 2: If the structure of the configuration storage of your editor does not fit your needs (e.g., because VS Code does not support settings.json being split in multiple files that can be committed), then this is a client issue.

@jsamr
Copy link
Author

jsamr commented Dec 11, 2021

Thank you for laying out those explanations! I will just add a few comments, with my limited understanding of the problem space:

it would be even more confusing that it already is to find out where a particular setting comes from, considering that editor settings, magic comments, Babel commands, external setting files, etc. all can influence the settings as well.

That's a fair point. It can be mitigated though by ignoring the editor settings when a config file is detected. A config file would imply this specific set of options is required for this workspace to maximize the chances of a reproducible execution of the program. Optionally, and to help end-users, the extension could report warnings when both editor and standalone configurations exists, and suggests deleting editor configuration entries.

the storage of the config is not [editor-agnostic], as configuration management is the responsibility of the client

With this proposal, the client (the extension) could choose to read a standalone config file, that would not break the LSP spec as far as I understand it.

If the structure of the configuration storage of your editor does not fit your needs (e.g., because VS Code does not support settings.json being split in multiple files that can be committed), then this is a client issue.

That holds true when the definition of "client" is restricted to the editor which has its own configuration scheme, and not the extension. Many VSCode extensions use standalone configuration files precisely because it has to be committed to an SCM, and because it cannot be editor-dependent.

To sum things up, I think there are two ways of considering the issue:

  1. In the client / server problem space and simplicity;
  2. In the configuration problem space, and more specifically along the axis "local / shared", "context-dependent / context-agnostic" and "individual taste / group conventions" ranges, which is the optic of online collaboration / SCM.

While writing my thoughts and doing some research, I realize that there is no consensus regarding committing .vscode/settings.json, mainly because although it is meant to be shared, it could store environment-sensitive entries such as absolute paths, and some extensions might add private information causing a major security issue. Worth to mention, there is also a proposal for recommended settings in VSCode.

In the end, I still think standalone configuration has merits for editor-independent online collaboration, but at the same time I do understand that you have to make design choices to limit complexity and stand by some philosophical guidelines to preserve consistency and quality in the long run. In any case, thank you for this amazing tool!

@valentjn
Copy link
Owner

valentjn commented Dec 12, 2021

Sorry, I didn't read your previous comment properly. You want the client to read configuration files and pass them on to the server. Yeah, that would be LSP-compliant. I guess I was confused because you opened this issue in the repo of ltex-ls, which is the server part and which doesn't know anything about editor extensions.

vscode-ltex already does this for some settings (dictionaries, hidden false positives, etc.) with the external setting files feature. The reason for that is that the values of these settings can become quite large, which would make the settings.json file messy. And even this feature was not present at the beginning, because for VS Code extensions, configuration management should be the responsibility of VS Code.

I think the request to be able to read in settings from external files other than settings.json (e.g., to be able to commit those, which is definitely a desirable feature) can be applied to almost any VS Code extension, even those that are not language clients. I wouldn't recommend that every extension of VS Code now starts implementing their own naming system (Which file name? In which directory? etc.), file format (JSON? YAML? etc.), handling of clashes (Does settings.json win? Or the external setting file(s)?), mechanism to read them in, etc. Even if some extensions might already be doing this themselves.

IMO, this is really something that should be implemented/supported by VS Code itself. Not sure if there's already an issue about this over at microsoft/vscode.

@valentjn valentjn added 2-upstream Issue status: Bug is caused by some dependency, might have to wait before continuing 3-not-our-issue Issue resolution: This issue cannot be solved in LTeX, but in some dependency labels Dec 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-question ❓ Issue type: General or specific question about LTeX 2-upstream Issue status: Bug is caused by some dependency, might have to wait before continuing 3-not-our-issue Issue resolution: This issue cannot be solved in LTeX, but in some dependency
Projects
None yet
Development

No branches or pull requests

2 participants