-
Notifications
You must be signed in to change notification settings - Fork 844
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
maven project compile args not passed to editor javac #1173
maven project compile args not passed to editor javac #1173
Conversation
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.
I left comments inline. There is one side question remaining: will the the compiler still work, if it is faced with unsupported parameters?
if (ARG_PARAMETERS.equals(compilerArg.trim())) { | ||
return Arrays.asList(ARG_PARAMETERS);//reqd? | ||
} | ||
return Arrays.asList(compilerArgs); |
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.
I don't see the point of this loop. Currently the code results in returning the full list of arguments, if at least one parameter is not null. I tried to provoke a NULL value in the arguments list. None of these variants resulted in a null value:
<arg></arg>
<arg/>
<arg xsi:nil="true" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"/>
So I would propose to return the argument list, if it is not null directly.
return Arrays.asList(ARG_PARAMETERS); | ||
if (compilerArg != null) { | ||
if (ARG_PARAMETERS.equals(compilerArg.trim())) { | ||
return Arrays.asList(ARG_PARAMETERS);//reqd? |
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.
In the original code there are only two cases:
- if a compiler argument
-parameters
was provided, it will be returned - else an empty list is returned
This makes sense reading the commit message of the addition:
http://hg.netbeans.org/main-golden/log/tip/maven/src/org/netbeans/modules/maven/queries/PomCompilerOptionsQueryImpl.java
It is basicly a guard to limit the change to that single parameter.
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.
Thanks, removed special handling for -parameters here.
validateCompilerOptions removes any unsupported parameters. |
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.
Looks good to me - thanks for the pointer to the validation code!
PS: Please squash the commits before merge.
No description provided.