search

c4spar / deno-cliffy

Command line framework for deno 🦕 Including Commandline-Interfaces, Prompts, CLI-Table, Arguments Parser and more...
https://cliffy.io
MIT License
955 stars 66 forks source link

External Commands on Windows is currently broken and will throw an "os error 123" #223

Closed rbeesley closed 3 years ago

rbeesley commented 3 years ago

https://github.com/c4spar/deno-cliffy/blob/d93ad7d8033dd5a630e6bad05722f96392e7c54e/command/command.ts#L1038-L1039

The mainModule path on Windows is file:///[DriveLetter]. With Linux and MacOS, there would be a root '/' when writing the path, but on Windows the path would begin with the drive letter. The replace function on Windows leaves a preceding slash so the result after the replacement is "/[DriveLetter]:/foo/bar.ts". When split, this leaves an empty string for the first part.

This causes https://github.com/c4spar/deno-cliffy/blob/d93ad7d8033dd5a630e6bad05722f96392e7c54e/command/command.ts#L1075 to throw os error 123, as it is trying to source "/[DriveLetter]:/foo/bar.ts" instead of "[DriveLetter]:/foo/bar.ts".

If you drop the preceding slash like was done with alephjs#201, for what appears to be the same problem, I believe this will fix the issue. Meanwhile on Windows, .executable() can't be used to call an external sub command.

c4spar commented 3 years ago

Hi @rbeesley, thanks for creating the issue. The externel commands are not very well tested at this time. But your problem should be easy to fix. I am currently unable to do a PR because I am sick. But if you want to do a PR, I'm happy to verify it. If not, I will do it as soon as I feel better.

rbeesley commented 3 years ago

I've made the first change and while working on source code this seems to be working. I anticipate that there may be additional fixes necessary to make this work once compiled as stand alone. Similarly I'm not sure how this will work if installed. With deno run however, this seems to be working now but since I'm not aware of the intricacies on another OS, and I'm not sure why there was the return false instead of continue, I don't know that I'd immediately commit. If there's more work to making commands work properly on Windows, I'll consider adding them to this bug report if it hasn't been committed upstream yet.

c4spar commented 3 years ago

@rbeesley i made some refactorings. the sub command needs to be installed globally now and the executable will be executed directly without deno run.