epugh commented on code in PR #3347:
URL: https://github.com/apache/solr/pull/3347#discussion_r2087142370
##########
solr/core/src/java/org/apache/solr/cli/SolrCLI.java:
##########
@@ -124,7 +124,7 @@ public static CommandLine parseCmdLine(Tool tool, String[]
args) {
List<String> dashDList = new ArrayList<>();
for (int a = 1; a < args.length; a++) {
String arg = args[a];
- if (arg.startsWith("-D")) {
+ if (!args[a - 1].equals("--jvm-opts") && arg.startsWith("-D")) {
Review Comment:
I think we probably hsould add some tests around this kind of tricky logic?
##########
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##########
@@ -723,12 +724,13 @@ protected Map<String, Object> startSolr(
forceArg,
verboseArg,
extraArgs,
- jvmOptsArg,
syspropArg);
startCmd = startCmd.replaceAll("\\s+", " ").trim(); // for pretty printing
+ String startCmdWithJvmOpts = startCmd + jvmOptsArg;
Review Comment:
I don't love this variable name... Maybe it's just `startCmd`? and a
comment why we moved `jvmOptsArg` out of the previous format command?
##########
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##########
@@ -723,12 +724,13 @@ protected Map<String, Object> startSolr(
forceArg,
verboseArg,
extraArgs,
- jvmOptsArg,
syspropArg);
startCmd = startCmd.replaceAll("\\s+", " ").trim(); // for pretty printing
+ String startCmdWithJvmOpts = startCmd + jvmOptsArg;
Review Comment:
Is this maybe not startCmd, but instead "About logging parameters"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
