* Allow gnuConfigureArgs to accept configure arguments with spaces. #292
Conversation
Currently, the arguments are split using space as a delimiter. With this change arguments containing spaces but specified within quotes are treated as a single argument.
|
Thanks for submitting this fix, @sraza1! Would you have time to create or modify one of the integration tests to illustrate this issue? The goal is to have a test which fails prior to your patch, but passes after it. |
Sure I'll get back with a test case shortly ... Thanks, |
| Matcher m = Pattern.compile("([^\"]\\S*|\".+?\")\\s*").matcher(this.gnuConfigureArgs); | ||
| // strip double quotes | ||
| while (m.find()) { | ||
| list.add(m.group(1).replace("\"", "")); |
There was a problem hiding this comment.
This seems a bit imprecise removing any "
Maybe you wouldn't use a " in gnuconfigure arg, or escape one in an arg?
There was a problem hiding this comment.
So, keeping in view #291 would searching for the starting " and ending " and stripping them only (if found) be the better way of doing this ?
There was a problem hiding this comment.
Sorry, totally forgot to come back to this.
Maybe I misread the regex, or it changed?
Could the args include an escaped \" quote? does it even need to be handled, I don't know.
taking the unit test and expanding with some other junk
<gnuConfigureArgs>
"--with-pkginfo=GNU Executable Integration Test" arg2 "arg 3"
"arg\" 4"
</gnuConfigureArgs>
Expected 4 args
--with-pkginfo=GNU Executable Integration Testarg2arg 3arg\" 4
Regex provided will come back with 6 matches
1 that's empty spaces/newlines
2-4 that need the leading/trailing " removed
5/6 should actually just be 5 and needing " removed.
I don't do much regex in Java, but I believe from doc this should work, and the capture group should already exclude the "
\s*\"(.*?)(?<!\\)\"\s* or as Java string "\\s*\\"(.*?)(?<!\\\\)\\"\\s*
Checked using https://regex101.com/
|
@sraza1 Are you still interested in submitting a test case to validate this bug-fix? It would be very much appreciated. |
|
@crtueden Yes, I would surely like to do that. Hopefully I'll be able to put up something during the next week... |
|
@ctrueden I have updated the PR for an integration test. Kindly take a look. Thanks, |
Currently, the arguments are split using space as a delimiter. With this change arguments
containing spaces but specified within quotes are treated as a single argument.