Skip to content

Substitute environment variables into config paths #265

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

KarelPeeters
Copy link
Contributor

Fixes #174.

I've tested this PR in the VSCode plugin using the Language Server User Path, and it works great for me.

@KarelPeeters
Copy link
Contributor Author

I'm not sure what the best way to handle non-existing environment variables is. Right now I just assume a default value of "" like bash without set -u, but maybe another option is better:

  • explicitly show an error to the user
  • silently ignore any paths that contain an undefined variable. This would also work as a convenient way to have optional libraries in the config file.

Copy link
Contributor

@Schottkyc137 Schottkyc137 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your contribution! This is a feature that many users will like.

I have added a few comments. None of them are blocking, so you may choose to address them but you can also ignore them. Just resolve the conversation so I know when you are ready.

@KarelPeeters
Copy link
Contributor Author

KarelPeeters commented Mar 2, 2024

I think I've addressed everything, let me know if there's anything else to improve!

@Schottkyc137 Schottkyc137 merged commit cb19f05 into VHDL-LS:master Mar 2, 2024
@Xcodo
Copy link
Contributor

Xcodo commented Mar 2, 2024

Thanks for doing this PR 👍. This was a problem for my team this week, fantastic to see a fix I can roll out soon!

@KarelPeeters
Copy link
Contributor Author

Thanks for the quick feedback and merge!

@KarelPeeters KarelPeeters deleted the subst-env branch March 3, 2024 10:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Add support to environment variable usage in the vhdl_ls.toml
4 participants