Escape filePath and args passed to the phpcs command #145
Conversation
|
@aslamdoctor thanks for the PR. Of course, both the filePath and the standard would need to be escaped if they're both in the same location 🤦 I'm annoyed I missed that 😁 One question: in my original PR, to fix for just the filePath, instead of running the string through a string replace, I wrap the string in double quotes. This comes from my experiences in bash scripting, where one should wrap any variables in double quotes, to prevent scripts from breaking when input contains spaces, line feeds, glob characters and such. I will freely admit I don't know a lot about the node child_process spawn method used to execute the phpcs command, but it seems to me that following the same process for escaping strings in the terminal made sense here. However, as it's a JavaScript code calling a method to execute the command, perhaps your solution is the better option. Do you have any opinions on this? @valeryan, as the original developer, I'd also appreciate any thoughts you might have on which solution is "better"; thanks. |
|
Seems like a good solution. Makes me wish I had time to write unit test though... |
|
@jonathanbossenger I was also confused why adding the double quote didn't work. It was supposed to work. It's quiet tedious to debug this. But I believe when you pass anything as argument to a command on terminal, it shouldn't have space and needs to be escaped with |
|
Ok, thanks for the confirming folks, let's go ahead and get this merged. |
@valeryan I hear you, but in my experience open source tools often don't get tests until someone comes along with enough time to add them. I will see if I can put some time aside for this as well. |
I can test this on Windows, hopefully tomorrow. |
Fixes issue #144
Tested on Mac.
Need to be tested on Windows.