-
Notifications
You must be signed in to change notification settings - Fork 541
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
fix command line parsing for all platforms #2009
base: master
Are you sure you want to change the base?
Conversation
I think this adds a significant readability problem to the code. Can we properly construct the command line string to be passed to the engine instead? The purpose of engine accepting a single |
First of all, I agree that the code is not pretty - especially with the preprocessor directives. Let's say we have the not so unusual case of spaces somewhere in the project path, like "flax projects\test1" and a very simple and portable code like this:
In Windows 11 cmd a test could look like that:
That means in the array it's clear that the space between flax and projects is part of the path whereas in the concatenated command line you have to guess.
or in the bash default way of escaping:
or in virtually every way you can imagine, including shell variables, command output and whatever. Now one could argue that the single arguments could be surrounded by quotes or special characters could be escaped.
Even in this exotic example the array preserves the information of what the path is.
So even Windows can list directories with special characters and it's not a Linux-specific problem. Let me show you another theoretical case that breaks the current logic. Assume somebody copies or moves a project and appends "-new" to it. Let's further assume this user is a bit clumsy on the keyboard and ends up with "test -new" i.e. another space. We end up with something like:
Now the -new part looks like an argument in the concatenated array but the user clearly stated that it's part of the name by using correct escape (quote) characters. What I try to make clear is that concatenating arguments will always bear the risk of losing information that is hardly recoverable in a reliable and portable way. My suggestion is that we change the entry point so it expects an array of strings and we can get rid of the confusing and maintenance-unfriendly preprocessor directives and implement just one command line handling for all. Another question is whether we want helpful error messages in case the arguments are used wrong. Things like "-project requires an argument" or "-porject is an invalid option" is best done when the structure is known beforehand. If those kind of checks and error reporting are not important than the parsing could be simplified a lot by going through the argument array just once. |
Please have a look at this proposal. It simplifies the command line parsing a lot while still preserving argument boundaries and thus still fixes the spaces-in-path problem. It can be made even shorter if the Windows version would supply an argument list as well. |
For systems using a posix-compliant shell command line arguments are parsed by the shell and relayed to the program via an array. The previous concatenation and parsing meant that escaped spaces were lost for Linux and OSX.