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

feat: Add Nushell support #389

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

colececil
Copy link
Contributor

@colececil colececil commented Jan 4, 2025

Implements #207

Though this implementation includes everything that needs done according to my understanding, it still doesn't seem to be working completely. I'd appreciate some help in figuring out what I might be missing. I believe the environment variables that are passed in to Export are being set correctly in the pre_prompt hook, but I'm not getting the expected SDKs and versions when I try to run them.

Also, note that this implementation wasn't quite as straightforward as I'd hoped, because Nushell doesn't have eval-like functionality. See https://www.nushell.sh/book/how_nushell_code_gets_run.html for more details, and see my comments below that point out how this impacts the implementation.

@colececil colececil marked this pull request as ready for review January 4, 2025 18:35
@bytemain
Copy link
Member

Will release 0.7.0 later today

@bytemain bytemain merged commit f14428d into version-fox:main Jan 24, 2025
@colececil
Copy link
Contributor Author

Thanks for reviewing this, @bytemain. Did you have any ideas as to why vfox didn't seem to be working for me in Nushell even with these changes, as I mentioned in my pull request summary above? Were you able to test it out on your machine?

@bytemain
Copy link
Member

First of all, I'd like to apologize. I didn't carefully review this PR that morning. I thought the function had already been implemented.
After some research, I've found some methods that can make it work better on NuShell. I'll submit a PR. By the way, NuShell is quite good to use.

@bytemain
Copy link
Member

bytemain commented Jan 25, 2025

The overall idea is on the right track. but the env -s nushell command yields an empty string (I'm not sure where this command is utilized, and it still returns an empty string even in a Zsh environment). In this case, we need to ensure that env -s nushell returns a valid environment.

@bytemain
Copy link
Member

fixed in #397

@colececil
Copy link
Contributor Author

@bytemain No worries. Thanks for following up on this and getting it to the finish line! I'll take a look at the changes you made in #397 and test them on my Windows machine.

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 this pull request may close these issues.

2 participants