groovy git commit: minor refactor: a little more consistency around how -Dname=value system properties are set (port for 2_4_X)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

groovy git commit: minor refactor: a little more consistency around how -Dname=value system properties are set (port for 2_4_X)

paulk
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_4_X 00b3161dd -> 233357b21


minor refactor: a little more consistency around how -Dname=value system properties are set (port for 2_4_X)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/233357b2
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/233357b2
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/233357b2

Branch: refs/heads/GROOVY_2_4_X
Commit: 233357b21b8bdc3f33621265f5abc4bea785b76d
Parents: 00b3161
Author: paulk <[hidden email]>
Authored: Thu Oct 12 18:19:53 2017 +1000
Committer: paulk <[hidden email]>
Committed: Thu Oct 12 18:44:03 2017 +1000

----------------------------------------------------------------------
 src/main/groovy/ui/GroovyMain.java              | 115 +++++--------------
 src/main/org/apache/groovy/util/SystemUtil.java |  46 ++++++++
 .../org/codehaus/groovy/tools/GrapeMain.groovy  |  11 +-
 .../groovy/tools/LoaderConfiguration.java       |  12 +-
 .../org/codehaus/groovy/tools/shell/Main.groovy |  29 ++---
 5 files changed, 89 insertions(+), 124 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/233357b2/src/main/groovy/ui/GroovyMain.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/ui/GroovyMain.java b/src/main/groovy/ui/GroovyMain.java
index b1aea2b..3b75e11 100644
--- a/src/main/groovy/ui/GroovyMain.java
+++ b/src/main/groovy/ui/GroovyMain.java
@@ -47,17 +47,14 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Properties;
 import java.util.regex.Pattern;
 
 /**
  * A Command line to execute groovy.
- *
- * @author Jeremy Rayner
- * @author Yuri Schimke
- * @author Roshan Dawrani
- * @version $Revision$
  */
 public class GroovyMain {
 
@@ -177,107 +174,46 @@ public class GroovyMain {
     @SuppressWarnings("static-access")
     private static synchronized Options buildOptions() {
         Options options = new Options();
-        options.addOption(OptionBuilder.hasArg().withArgName("path").withDescription("Specify where to find the class files - must be first argument").create("classpath"));
-        options.addOption(OptionBuilder.withLongOpt("classpath").hasArg().withArgName("path").withDescription("Aliases for '-classpath'").create("cp"));
-
         options.addOption(
-            OptionBuilder.withLongOpt("define").
-            withDescription("define a system property").
-            hasArg(true).
-            withArgName("name=value").
-            create('D'));
+            OptionBuilder.hasArg().withArgName("path").withDescription("Specify where to find the class files - must be first argument").create("classpath"));
+        options.addOption(
+            OptionBuilder.withLongOpt("classpath").hasArg().withArgName("path").withDescription("Aliases for '-classpath'").create("cp"));
+        options.addOption(
+            OptionBuilder.withLongOpt("define").withDescription("define a system property").hasArgs(2).withValueSeparator().withArgName("name=value").create('D'));
         options.addOption(
             OptionBuilder.withLongOpt("disableopt").
             withDescription("disables one or all optimization elements. " +
                             "optlist can be a comma separated list with the elements: " +
                             "all (disables all optimizations), " +
                             "int (disable any int based optimizations)").
-            hasArg(true).
-            withArgName("optlist").
-            create());
+            hasArg(true).withArgName("optlist").create());
         options.addOption(
-            OptionBuilder.hasArg(false)
-            .withDescription("usage information")
-            .withLongOpt("help")
-            .create('h'));
+            OptionBuilder.hasArg(false).withDescription("usage information").withLongOpt("help").create('h'));
         options.addOption(
-            OptionBuilder.hasArg(false)
-            .withDescription("debug mode will print out full stack traces")
-            .withLongOpt("debug")
-            .create('d'));
+            OptionBuilder.hasArg(false).withDescription("debug mode will print out full stack traces").withLongOpt("debug").create('d'));
         options.addOption(
-            OptionBuilder.hasArg(false)
-            .withDescription("display the Groovy and JVM versions")
-            .withLongOpt("version")
-            .create('v'));
+            OptionBuilder.hasArg(false).withDescription("display the Groovy and JVM versions").withLongOpt("version").create('v'));
         options.addOption(
-            OptionBuilder.withArgName("charset")
-            .hasArg()
-            .withDescription("specify the encoding of the files")
-            .withLongOpt("encoding")
-            .create('c'));
+            OptionBuilder.withArgName("charset").hasArg().withDescription("specify the encoding of the files").withLongOpt("encoding").create('c'));
         options.addOption(
-            OptionBuilder.withArgName("script")
-            .hasArg()
-            .withDescription("specify a command line script")
-            .create('e'));
+            OptionBuilder.withArgName("script").hasArg().withDescription("specify a command line script").create('e'));
         options.addOption(
-            OptionBuilder.withArgName("extension")
-            .hasOptionalArg()
-            .withDescription("modify files in place; create backup if extension is given (e.g. \'.bak\')")
-            .create('i'));
+            OptionBuilder.withArgName("extension").hasOptionalArg().withDescription("modify files in place; create backup if extension is given (e.g. \'.bak\')").create('i'));
         options.addOption(
-            OptionBuilder.hasArg(false)
-            .withDescription("process files line by line using implicit 'line' variable")
-            .create('n'));
+            OptionBuilder.hasArg(false).withDescription("process files line by line using implicit 'line' variable").create('n'));
         options.addOption(
-            OptionBuilder.hasArg(false)
-            .withDescription("process files line by line and print result (see also -n)")
-            .create('p'));
+            OptionBuilder.hasArg(false).withDescription("process files line by line and print result (see also -n)").create('p'));
         options.addOption(
-            OptionBuilder.withArgName("port")
-            .hasOptionalArg()
-            .withDescription("listen on a port and process inbound lines (default: 1960)")
-            .create('l'));
+            OptionBuilder.withArgName("port").hasOptionalArg().withDescription("listen on a port and process inbound lines (default: 1960)").create('l'));
         options.addOption(
-            OptionBuilder.withArgName("splitPattern")
-            .hasOptionalArg()
-            .withDescription("split lines using splitPattern (default '\\s') using implicit 'split' variable")
-            .withLongOpt("autosplit")
-            .create('a'));
+            OptionBuilder.withArgName("splitPattern").hasOptionalArg().withDescription("split lines using splitPattern (default '\\s') using implicit 'split' variable").withLongOpt("autosplit").create('a'));
         options.addOption(
-            OptionBuilder.withLongOpt("indy")
-            .withDescription("enables compilation using invokedynamic")
-            .create());
+            OptionBuilder.withLongOpt("indy").withDescription("enables compilation using invokedynamic").create());
         options.addOption(
-            OptionBuilder.withLongOpt("configscript")
-            .hasArg().withDescription("A script for tweaking the configuration options")
-            .create());
+            OptionBuilder.withLongOpt("configscript").hasArg().withDescription("A script for tweaking the configuration options").create());
         options.addOption(
-                OptionBuilder.withLongOpt("basescript")
-                .hasArg().withArgName("class").withDescription("Base class name for scripts (must derive from Script)")
-                .create('b'));
+            OptionBuilder.withLongOpt("basescript").hasArg().withArgName("class").withDescription("Base class name for scripts (must derive from Script)").create('b'));
         return options;
-
-    }
-
-    private static void setSystemPropertyFrom(final String nameValue) {
-        if(nameValue==null) throw new IllegalArgumentException("argument should not be null");
-
-        String name, value;
-        int i = nameValue.indexOf("=");
-
-        if (i == -1) {
-            name = nameValue;
-            value = Boolean.TRUE.toString();
-        }
-        else {
-            name = nameValue.substring(0, i);
-            value = nameValue.substring(i + 1, nameValue.length());
-        }
-        name = name.trim();
-
-        System.setProperty(name, value);
     }
 
     /**
@@ -290,10 +226,11 @@ public class GroovyMain {
         List args = line.getArgList();
         
         if (line.hasOption('D')) {
-            String[] values = line.getOptionValues('D');
-
-            for (int i=0; i<values.length; i++) {
-                setSystemPropertyFrom(values[i]);
+            Properties optionProperties = line.getOptionProperties("D");
+            Enumeration<String> propertyNames = (Enumeration<String>) optionProperties.propertyNames();
+            while (propertyNames.hasMoreElements()) {
+                String nextName = propertyNames.nextElement();
+                System.setProperty(nextName, optionProperties.getProperty(nextName));
             }
         }
 

http://git-wip-us.apache.org/repos/asf/groovy/blob/233357b2/src/main/org/apache/groovy/util/SystemUtil.java
----------------------------------------------------------------------
diff --git a/src/main/org/apache/groovy/util/SystemUtil.java b/src/main/org/apache/groovy/util/SystemUtil.java
new file mode 100644
index 0000000..967b37b
--- /dev/null
+++ b/src/main/org/apache/groovy/util/SystemUtil.java
@@ -0,0 +1,46 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.groovy.util;
+
+public class SystemUtil {
+    /**
+     * Sets a system property from a name=value String
+     *
+     * @param nameValue the non-null name=value String
+     * @return the found name
+     */
+    public static String setSystemPropertyFrom(final String nameValue) {
+        if (nameValue == null) throw new IllegalArgumentException("argument should not be null");
+
+        String name, value;
+        int i = nameValue.indexOf("=");
+
+        if (i == -1) {
+            name = nameValue;
+            value = Boolean.TRUE.toString();
+        } else {
+            name = nameValue.substring(0, i);
+            value = nameValue.substring(i + 1, nameValue.length());
+        }
+        name = name.trim();
+
+        System.setProperty(name, value);
+        return name;
+    }
+}

http://git-wip-us.apache.org/repos/asf/groovy/blob/233357b2/src/main/org/codehaus/groovy/tools/GrapeMain.groovy
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/tools/GrapeMain.groovy b/src/main/org/codehaus/groovy/tools/GrapeMain.groovy
index 0a1843d..85d8a4a 100644
--- a/src/main/org/codehaus/groovy/tools/GrapeMain.groovy
+++ b/src/main/org/codehaus/groovy/tools/GrapeMain.groovy
@@ -276,7 +276,8 @@ import org.apache.commons.cli.*
 options.addOption(
     OptionBuilder.withLongOpt("define")
         .withDescription("define a system property")
-        .hasArg(true)
+        .hasArgs(2)
+ .withValueSeparator()
         .withArgName("name=value")
         .create('D')
 );
@@ -348,10 +349,10 @@ if (cmd.hasOption('v')) {
     return
 }
 
-
-cmd.getOptionValues('D')?.each {String prop ->
-    def (k, v) = prop.split ('=', 2) as List // array multiple assignment quirk
-    System.setProperty(k, v ?: "")
+if (options.hasOption('D')) {
+    options.getOptionProperties('D')?.each { k, v ->
+        System.setProperty(k, v)
+    }
 }
 
 String[] arg = cmd.args

http://git-wip-us.apache.org/repos/asf/groovy/blob/233357b2/src/main/org/codehaus/groovy/tools/LoaderConfiguration.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/tools/LoaderConfiguration.java b/src/main/org/codehaus/groovy/tools/LoaderConfiguration.java
index ffcb8da..a43ece8 100644
--- a/src/main/org/codehaus/groovy/tools/LoaderConfiguration.java
+++ b/src/main/org/codehaus/groovy/tools/LoaderConfiguration.java
@@ -18,6 +18,8 @@
  */
 package org.codehaus.groovy.tools;
 
+import org.apache.groovy.util.SystemUtil;
+
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
@@ -72,7 +74,6 @@ import java.util.regex.Pattern;
  * not exist, an empty string will be used. If the path or file after the load
  * command does not exist, the path will be ignored.
  *
- * @author Jochen Theodorou
  * @see RootLoader
  */
 public class LoaderConfiguration {
@@ -126,13 +127,8 @@ public class LoaderConfiguration {
                 main = line.substring(MAIN_PREFIX.length()).trim();
             } else if (line.startsWith(PROP_PREFIX)) {
                 String params = line.substring(PROP_PREFIX.length()).trim();
-                int index = params.indexOf('=');
-                if (index == -1) {
-                    throw new IOException("unexpected property format - expecting name=value" + lineNumber + " : " + line);
-                }
-                String propName = params.substring(0, index);
-                String propValue = assignProperties(params.substring(index + 1));
-                System.setProperty(propName, propValue);
+                String key = SystemUtil.setSystemPropertyFrom(params);
+                System.setProperty(key, assignProperties(System.getProperty(key)));
             } else {
                 throw new IOException("unexpected line in " + lineNumber + " : " + line);
             }

http://git-wip-us.apache.org/repos/asf/groovy/blob/233357b2/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Main.groovy
----------------------------------------------------------------------
diff --git a/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Main.groovy b/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Main.groovy
index 5624781..289cf89 100644
--- a/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Main.groovy
+++ b/subprojects/groovy-groovysh/src/main/groovy/org/codehaus/groovy/tools/shell/Main.groovy
@@ -29,6 +29,8 @@ import org.codehaus.groovy.tools.shell.util.NoExitSecurityManager
 import org.fusesource.jansi.Ansi
 import org.fusesource.jansi.AnsiConsole
 
+import static org.apache.groovy.util.SystemUtil.setSystemPropertyFrom
+
 /**
  * A Main instance has a Groovysh member representing the shell,
  * and a startGroovysh() method to run an interactive shell.
@@ -42,8 +44,6 @@ import org.fusesource.jansi.AnsiConsole
  * static ansi state using the jAnsi library.
  *
  * Main CLI entry-point for <tt>groovysh</tt>.
- *
- * @author <a href="mailto:[hidden email]">Jason Dillon</a>
  */
 class Main
 {
@@ -78,7 +78,7 @@ class Main
             d(longOpt: 'debug', messages['cli.option.debug.description'])
             e(longOpt: 'evaluate', args: 1, argName: 'CODE', optionalArg: false, messages['cli.option.evaluate.description'])
             C(longOpt: 'color', args: 1, argName: 'FLAG', optionalArg: true, messages['cli.option.color.description'])
-            D(longOpt: 'define', args: 1, argName: 'NAME=VALUE', messages['cli.option.define.description'])
+            D(longOpt: 'define', args: 2, argName: 'name=value', valueSeparator: '=', messages['cli.option.define.description'])
             T(longOpt: 'terminal', args: 1, argName: 'TYPE', messages['cli.option.terminal.description'])
         }
         OptionAccessor options = cli.parse(args)
@@ -123,10 +123,8 @@ class Main
         IO io = new IO()
 
         if (options.hasOption('D')) {
-            def values = options.getOptionValues('D')
-
-            values.each {
-                setSystemProperty(it as String)
+            options.getOptionProperties('D')?.each { k, v ->
+                System.setProperty(k, v)
             }
         }
 
@@ -248,22 +246,9 @@ class Main
         Ansi.setDetector(new AnsiDetector())
     }
 
-
+    @Deprecated
     static void setSystemProperty(final String nameValue) {
-        String name
-        String value
-
-        if (nameValue.indexOf('=') > 0) {
-            def tmp = nameValue.split('=', 2)
-            name = tmp[0]
-            value = tmp[1]
-        }
-        else {
-            name = nameValue
-            value = Boolean.TRUE.toString()
-        }
-
-        System.setProperty(name, value)
+        setSystemPropertyFrom(nameValue)
     }
 }