-
Notifications
You must be signed in to change notification settings - Fork 233
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
Fixed problems with shellslash #363
Conversation
This should fix some problems that I found on windows when using the `shellslash` option to use forward slashes instead of backslashes.
Found a problem: on linux the shellslash option is not disabled, it doesn't exists at all, and the code breaks. I'm making a change on that regard (using pcall to handle the eventual error), but I'm not sure how to update the PR. |
Tested on my linux laptop, that issue should be fixed now |
Hi @andreadev-it, thanks for submitting this PR. I'm OK with it as is, but I'm going to wait and see what happens on Plenary before merging. If it is accepted there and turned into a utility function, it might be better to just reuse that final Plenary implementation here instead of duplicating it. |
Hi @andreadev-it, it looks like the plenary one is stalled and I don't think it is relevant anymore because I have stopped using plenary for reading the filesystem. Are you still interested in merging this? I have to say, it seems like there should be more code changes/refactoring to make this work universally. I did a reference search for |
Hi @cseickel , Personally, I've stopped using shellslash, but if you think there might be the need to adjust it, I can look it up a little deeper and try to come up with a better fix |
No problem @andreadev-it. No one else has asked for shellslash related improvements, so if you are no longer using it then I will close this PR. |
These changes should fix most of the issue about using shellslash on windows. However, maybe it's better to wait for this PR on plenary to be merged before accepting this. Without the changes on plenary, many things will just not work anyway.