Don't quote input/output when invoking external formatters #99

xitep commented 2025年11月13日 18:51:37 +01:00

On unix/linux, the hard coded quotes around the input/output file names get passed (as part of the filenames) to my external formatter, which then is unable to open the files (since the quotes are not part of their name.) Unfortunately, I don't have access to a windows machine, so I'm not able to see if it possibly breaks anything there. It shouldn't. ProcessBuilder gets the precise list of arguments and should not do any further tokenisation.

A shell script as simple as the following can be used to test it (ie. to simply echo back the arguments to wb again):

#!/bin/sh

for i in "$@" ; do
 echo ">$i<"
done

ps: this issue made me upgrade my quite outdated wb installation 😊 ... many thanks for keeping it up and maintained all those years! 🙇

On unix/linux, the hard coded quotes around the input/output file names get passed (as part of the filenames) to my external formatter, which then is unable to open the files (since the quotes are not part of their name.) Unfortunately, I don't have access to a windows machine, so I'm not able to see if it possibly breaks anything there. It shouldn't. `ProcessBuilder` gets the precise list of arguments and should not do any further tokenisation. A shell script as simple as the following can be used to test it (ie. to simply echo back the arguments to wb again): ```sh #!/bin/sh for i in "$@" ; do echo ">$i<" done ``` ps: this issue made me upgrade my quite outdated wb installation 😊 ... many thanks for keeping it up and maintained all those years! 🙇

sql-workbench commented 2025年11月15日 10:34:25 +01:00

Thanks for the feedback.

On Windows the quoting is most certainly necessary, I will check if there is a universal way of doing that. Maybe by calling ProcessBuilder in a different way.

I also wonder if quoting is necessary if the temp file written ends up in a directory containing spaces - which would require quoting on Linux as well I assume.

Thanks for the feedback. On Windows the quoting is most certainly necessary, I will check if there is a universal way of doing that. Maybe by calling ProcessBuilder in a different way. I also wonder if quoting is necessary if the temp file written ends up in a directory containing spaces - which would require quoting on Linux as well I assume.

xitep commented 2025年11月15日 10:49:51 +01:00

the "trick" i applied (and hope would work for windows) is explained here: https://stackoverflow.com/a/12124972

the "trick" i applied (and hope would work for windows) is explained here: https://stackoverflow.com/a/12124972

xitep commented 2025年11月15日 10:52:37 +01:00

I also wonder if quoting is necessary if the temp file written ends up in a directory containing spaces

no, it'll work. indeed, i tested with ${wbin} expanding into a string with a space (i just changed the file template directly in the code.) such a temp file gets then correctly passed to the executable as one argument. (of course, it's then up to the program to not break it up further by whitespace.)

> I also wonder if quoting is necessary if the temp file written ends up in a directory containing spaces no, it'll work. indeed, i tested with `${wbin}` expanding into a string with a space (i just changed the file template directly in the code.) such a temp file gets then correctly passed to the executable as one argument. (of course, it's then up to the program to not break it up further by whitespace.)

sql-workbench commented 2025年11月15日 12:16:24 +01:00

A command line like ${wbin} will indeed work. But for a program that requires something like inputFile=${wbin} the filename has to be quoted (especially if there is another parameter after the inputFile)

A command line like `${wbin}` will indeed work. But for a program that requires something like `inputFile=${wbin}` the filename has to be quoted (especially if there is _another_ parameter after the `inputFile`)

xitep commented 2025年11月15日 12:41:41 +01:00

and would "inputFile=${wbin}" anotherParameter work? this is, the user explicitly quoting what belongs together and what is seen as separate arguments.

and would `"inputFile=${wbin}" anotherParameter` work? this is, the user explicitly quoting what belongs together and what is seen as separate arguments.

sql-workbench commented 2025年11月16日 17:29:49 +01:00

I decided to solve this by parsing the command line in a way that preserves any double quotes provided by the user.

Therefor I will close this PR (which I have integrated partly into my solution)

I decided to solve this by parsing the command line in a way that preserves any double quotes provided by the user. Therefor I will close this PR (which I have integrated partly into my solution)

xitep commented 2025年11月16日 17:53:01 +01:00

wonderful, works like a charm on my system! you even managed to preserve user provided quotes 👍 many thanks for the solution! 🙇

wonderful, works like a charm on my system! you even managed to preserve user provided quotes 👍 many thanks for the solution! 🙇