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

Allow using relative paths for settings #105

Closed
flying-sheep opened this issue Nov 8, 2021 · 6 comments · Fixed by #106
Closed

Allow using relative paths for settings #105

flying-sheep opened this issue Nov 8, 2021 · 6 comments · Fixed by #106

Comments

@flying-sheep
Copy link
Contributor

flying-sheep commented Nov 8, 2021

There’s two possible ways to go about this:

One step in the resolution of #71

@flying-sheep flying-sheep changed the title Allow using some variable substitutions in the path settings Allow using relative paths for settings Nov 8, 2021
@flying-sheep
Copy link
Contributor Author

@sindresorhus which one would you prefer? I could do a quick PR.

@spence-s
Copy link
Collaborator

spence-s commented Nov 8, 2021

I'm willing to support/merge this if you want to make a PR @flying-sheep seems like a reasonable use case to me.

The only line that would need to be changed is this:
https://github.com/xojs/vscode-linter-xo/blob/master/server/server.js#L464

I don't think we should use the same variable substitution that vscode uses, because I don't think we need to support any token other than workspace folder. Maybe a token like?
"path": "WORKSPACE_FOLDER/path/to/xo"

A relative path would be fine too - it just needs to be clear in the docs that if you supply an absolute path it must be in file uri format and the path will always be relative to the folder root.

Will need to change this as well:
https://github.com/xojs/vscode-linter-xo/blob/master/package.json#L82

@flying-sheep
Copy link
Contributor Author

I’d prefer a relative path to a custom placeholder. The ${workspaceFolder} idea was because it’s familiar, but so is using a relative path (e.g. for eslint.nodePath)

@spence-s
Copy link
Collaborator

spence-s commented Nov 8, 2021

sounds good @flying-sheep - I will take care of this later today If I don't see a PR.

@spence-s
Copy link
Collaborator

spence-s commented Nov 8, 2021

@flying-sheep this is published now 3.7.0 - I went ahead an added a couple of lines to deprecate the file uri and made the option take an absolute path or a relative path because I felt this made more sense. Thanks for your help!

@flying-sheep
Copy link
Contributor Author

great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants