-
Notifications
You must be signed in to change notification settings - Fork 158
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 the bug of spliting strings of configuration values. #146
base: master
Are you sure you want to change the base?
Conversation
NAR plugin for Maven » nar-maven-plugin #283 SUCCESS |
…iguration, and also change the order of setting defines and undefines.
NAR plugin for Maven » nar-maven-plugin #284 SUCCESS |
Thank you for your contribution! I offered some suggestions for improvements, hopefully I was clear. If not, or if you disagree, please do not hesitate to answer my concerns. Again, thanks for contributing to the NAR plugin! |
About the unit test. |
I thought more about an integration test. Example: https://github.com/maven-nar/nar-maven-plugin/tree/master/src/it/it0003-jni |
@Haixing-Hu still working on it? |
sorry that I'm working on a company project and was very busy these days. I will continue work on the nar-maven-plugin project in my spare time.
|
We all do... All the contributors to the |
Some configuration parameters accept the space or comma separated values. For example, the
defaultIncludes
,defaultExcludes
,optionSet
,defineSet
,undefineSet
, and thecompileOrder
.The
Compiler.getCompiler()
function will split those values and add them to the list of compiler arguments.But the implementation of
Compiler.getCompiler()
failed to filter the empty sub-strings, and failed to trim the sub-strings, produced by the splitting.This patch simply uses a regular expression as the separator to filter the empty sub-strings and trim the resulting sub-strings.