-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Bootstrap Maven: support for MAVEN_OPTS and properties with whitespaces #12013
Conversation
@gsmet fyi, with this one you won't have to add |
@aloubyansky whatever you think is best. Should I close my PR, then? |
@metacosm i think so, given that this one covers your issue plus another one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 2 questions.
for (Object o : userProps.keySet()) { | ||
String name = o.toString(); | ||
final String value = userProps.getProperty(name); | ||
buf.setLength(2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this go away?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. What made you question that?
buf.append(name); | ||
if (value != null && !value.isEmpty()) { | ||
buf.append('='); | ||
buf.append(value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to escape the values? They will be escaped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should work. This PR includes a devmode test passing a system property with a value containing whitespaces using -D.
Fixes #11819
Fixes #11467
@metacosm fyi, I took a bit of a different approach here.