-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: added support for NixOS config files #229
feat: added support for NixOS config files #229
Conversation
Hi @KhanKudo thank you for your contribution. First look looks good. Nicely isolated. I will try to give a better look in the next week. |
Awesome, glad to hear you like it. It would be fantastic to close the feature request. This behavior has been wished for since at least January. Given the large user base of Path-Intellisense, I tried my best to make sure there were no unintended consequences, thus the selector limits use to explicitly .nix files. The implementation however could very well be useful in .sh or other text files where paths are commonly used outside of quoted strings, such as for GitLab/GitHub CI/CD configs. |
Hi @KhanKudo I reviewed and tested your PR. It works as expected and the code is easy to understand. I like how you added it as a separate provider and made sure it doesn't conflict with the existing solution. Thank you for your amazing work 🤩 I noticed that you are reusing the Could you duplicate code? It is a few duplicates line but mostly just calls to utils fns. The file Note: the test are not running on Ci. This has nothing to do with your change. I have to check what the issue. Thank you again for your contribution. Looking forward to merge the first provider next to JS |
I absolutely agree, different providers should not depend on each other. I have pushed a new commit that moves |
Thank you @KhanKudo for making those changes. Looks very good. The only thing you missed is to revert the changes in the javascript provider in the |
Oh that's a silly oversight haha. |
Thank you @KhanKudo for your contribution. Nice work and pleasant discussion. I will create a new release with your PR. 🚀 |
# [2.9.0](v2.8.0...v2.9.0) (2024-06-01) ### Features * added support for NixOS config files ([#229](#229)) ([6aa458c](6aa458c))
This PR resolves the feature request from the official vscode-nix-ide extension repository. (Closes nix-community/vscode-nix-ide#379)
It doesn't change any of the existing behavior, only extends it by adding a new provider for **/*.nix file pattern. This provider has also been placed after the javascript provider to ensure it never overwrites it.
The primary complexity of implementing this feature was the fact that in .nix configs file paths are primarily placed outside of "quotes" resulting in the default vscode context.fromString property being empty, as there is no string to recognize. A custom short regex has been used to match the expected behavior of context.fromString in cases without quotation marks. Worth noting is that, if the path is placed in a string, the default result of context.fromString is forwarded, without invoking any of the custom regex recognition.