-
Notifications
You must be signed in to change notification settings - Fork 0
Support -D system properties for compiler options (#275) #29
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
Changes from all commits
ab8a052
e30c859
9f012cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,17 +52,25 @@ public CompilerArgumentsReader(ProcessingEnvironment processingEnv) { | |
| /** | ||
| * Reads the value of a compiler argument. | ||
| * | ||
| * <p>The method looks up the compiler argument using both the full compiler argument name (with | ||
| * prefix) and the simple option name (without prefix) for backward compatibility. | ||
| * <p>The method checks the prefixed JVM system property first, then the prefixed compiler | ||
| * argument, and finally the bare option name for backward compatibility. The system property wins | ||
| * so a command-line {@code -D} can override options configured in the build file. The system | ||
| * property is available when the build tool runs javac in-process and is not available with | ||
| * {@code <fork>true</fork>}. | ||
| * | ||
| * @param argument the compiler argument enum to read | ||
| * @return the value of the compiler argument, or null if not set | ||
| */ | ||
| public String readValue(CompilerArgumentsEnum argument) { | ||
| // Try with full compiler argument name first (e.g., "simplebuilder.verbose") | ||
| String value = processingEnv.getOptions().get(argument.getCompilerArgument()); | ||
| // Try the -D JVM system property first (e.g., -Dsimplebuilder.verbose) | ||
| String value = System.getProperty(argument.getCompilerArgument()); | ||
|
|
||
| // Fall back to simple option name for backward compatibility (e.g., "verbose") | ||
| // Then the -A compiler argument (e.g., -Asimplebuilder.verbose) | ||
| if (value == null) { | ||
| value = processingEnv.getOptions().get(argument.getCompilerArgument()); | ||
| } | ||
|
|
||
| // Finally the bare option name for backward compatibility (e.g., -Averbose) | ||
| if (value == null) { | ||
| value = processingEnv.getOptions().get(argument.getOptionName()); | ||
| } | ||
|
|
@@ -129,11 +137,18 @@ public AccessModifier readAccessModifier(CompilerArgumentsEnum argument) { | |
| * <p>This method reads all configuration options from compiler arguments like: | ||
| * | ||
| * <ul> | ||
| * <li>{@code -Asimplebuilder.generateFieldSupplier=true} | ||
| * <li>{@code -Dsimplebuilder.generateFieldSupplier=true} (JVM system property, highest | ||
| * precedence) | ||
| * <li>{@code -Asimplebuilder.generateFieldSupplier=true} (compiler argument) | ||
| * <li>{@code -AgenerateFieldSupplier=true} (bare option name, backward compatibility) | ||
| * <li>{@code -Asimplebuilder.builderAccess=public} | ||
| * <li>etc. | ||
|
Comment on lines
139
to
145
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -D first, because it is handled first? And what about the option name? And where are the option from
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, |
||
| * </ul> | ||
| * | ||
| * <p>Options set via {@code @SimpleBuilder.Options} on the annotated type are not handled here; | ||
| * they are read by {@link BuilderConfigurationReader} and merged on top of this global | ||
| * configuration. | ||
| * | ||
| * <p>All values default to UNSET or DEFAULT if not specified in compiler arguments. | ||
| * | ||
| * <p><b>Adding a new option:</b> every option in {@link CompilerArgumentsEnum} that represents a | ||
|
|
||
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 is not the fallback, it is the default. And it is not maven only.
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.
Fixed: now "the same name also works as a JVM system property (
-Dsimplebuilder.<option>), which takes precedence over-A" — no "fallback", no Maven-only wording (9f012cd).