jorgebucaran / nvm.fish

The Node.js version manager you'll adore, crafted just for Fish
https://git.io/nvm.fish
MIT License
2.06k stars 69 forks source link

Fixed MINGW/MSYS2 OS detection and zip extraction #192

Open matteopt opened 2 years ago

jorgebucaran commented 1 year ago

Hey, thanks a lot for your contribution! Sorry for the delay in reviewing it. I'm having a hard time finding time to test it properly, so I can't move forward at this time. If someone with more free time and a Windows terminal can help review, that would be amazing and I'd be happy to take another look. 🙏

Ping @tasop-rsconnect @M4RC3L05

M4RC3L05 commented 1 year ago

Hey, thanks a lot for your contribution! Sorry for the delay in reviewing it. I'm having a hard time finding time to test it properly, so I can't move forward at this time. If someone with more free time and a Windows terminal can help review, that would be amazing and I'd be happy to take another look. 🙏

Ping @tasop-rsconnect @M4RC3L05

Working on a clean msys2 install using mingw64 shell Screenshot 2023-04-30 101626

Should we still change the readme to have it state that for msys2 to check for either unzip or bsdtar. Just to make sure that the user allways check to make sure it works, idk, wdyt? @jorgebucaran @paramtt

jorgebucaran commented 1 year ago

Yeah, we should definitely add that to the system requirements. But hey, I did a quick search and it looks like Windows has this built-in expand command that's always there. Maybe we could use that instead? Or is it not the same thing?

M4RC3L05 commented 1 year ago

Yeah, we should definitely add that to the system requirements. But hey, I did a quick search and it looks like Windows has this built-in expand command that's always there. Maybe we could use that instead? Or is it not the same thing?

Since this is running on msys2 the command expand gets mapped to the linux conterpart, to convert tabs to spaces, it is a different shell from cmd or powershell. As @paramtt said on the issue linked to this PR it fallbacks to bsdtar that comes bundled. I believe that it is ok to use that.