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
resolvePath() can't return more than one path #43
Comments
That's because you are supposed to return the whole token with globbing expanded. E.g. if you receive as argument
You're right, I thought about the same issue regarding command substitution (that's also async in nature). The solution is not an easy one, because all the parser has to become asyncronous... |
That's what I thought but it seems I end up with just one argument of
No, I think you're right in that the parser has to become asynchronous and I agree that it's not a simple change. Thanks. |
Why to return a formatted string? Wouldn't it be better and simpler to return an array instead? It could be converted to an string too...
I would move towards that path... :-)
Async / await, maybe? |
Why not return an array, such as |
Right, I'll made such change... or someone want to do a PR 😸 ? Anyone know of a good npm module to escape file names? There could be more to escape than spaces, e.g.:
I ❤️ async/await, it could greatly simplify asynchronous code... but I don't wnat to setup a transpilation, so that mean that we can support only node > 7. @nfischer what about |
I don't mind bumping up the support to 7. Cash is a front-end library, so it doesn't drag monolithic projects with it that can't upgrade so easily. |
👍 @dthree, so I will start to use syntax construct only available in node >= 7.4 in the process of introducing async behavior in the parser. |
I don't see why this module needs to escape things for the shell. Did you mean parsing? Most modules would prefer the un-escaped file names (e.g. A |
No, I really mean escape, and that's not to pass escaped version of the file names to child process, it's because I have to apply field splitting to the result of globbing, because file names has to be joined in a single string and later pass to child process as an array. But after more thorough thinking I'm not sure that's really necessary... |
I don't see why it would be necessary. To me, it sounds like the worse option: we would be requiring users to use unsafe APIs like |
Looks like
resolvePath()
can't return more than one path, this makes globbing*
hard since that will likely result in more than one path. I'm currently working around this by returning a magic value and then doing globbing when I encounter the value in the AST but it seems this should be handled by bash-parser internally. In addition, globbing might involve file system access and as such it would be useful if it could be done asynchronously. Right now the value needs to be returned directly from resolvePath().Thanks!
The text was updated successfully, but these errors were encountered: