[1/2] groovy git commit: minor refactor: a little more consistency around how -Dname=value system properties are set

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

[1/2] groovy git commit: minor refactor: a little more consistency around how -Dname=value system properties are set

paulk
Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_6_X 2974831c1 -> c4e5e2250


minor refactor: a little more consistency around how -Dname=value system properties are set


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

Branch: refs/heads/GROOVY_2_6_X
Commit: c4e5e22505f7eafa384bb802d83d6a9c40faf086
Parents: 8e9b3ec
Author: paulk <[hidden email]>
Authored: Thu Oct 12 18:19:53 2017 +1000
Committer: paulk <[hidden email]>
Committed: Thu Oct 12 18:21:04 2017 +1000

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


http://git-wip-us.apache.org/repos/asf/groovy/blob/c4e5e225/src/main/groovy/ui/GroovyMain.java
----------------------------------------------------------------------
diff --git a/src/main/groovy/ui/GroovyMain.java b/src/main/groovy/ui/GroovyMain.java
index 402d53e..04c5a14 100644
--- a/src/main/groovy/ui/GroovyMain.java
+++ b/src/main/groovy/ui/GroovyMain.java
@@ -46,18 +46,16 @@ 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;
 
 import static org.apache.commons.cli.Option.builder;
 
 /**
  * A Command line to execute groovy.
- *
- * @author Jeremy Rayner
- * @author Yuri Schimke
- * @author Roshan Dawrani
  */
 public class GroovyMain {
 
@@ -178,49 +176,30 @@ public class GroovyMain {
         return new Options()
                 .addOption(builder("classpath").hasArg().argName("path").desc("Specify where to find the class files - must be first argument").build())
                 .addOption(builder("cp").longOpt("classpath").hasArg().argName("path").desc("Aliases for '-classpath'").build())
-                .addOption(builder("D").longOpt("define").desc("define a system property").hasArg().argName("name=value").build())
+                .addOption(builder("D").longOpt("define").desc("Define a system property").numberOfArgs(2).valueSeparator().argName("name=value").build())
                 .addOption(
                         builder().longOpt("disableopt")
-                                .desc("disables one or all optimization elements. " +
+                                .desc("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().argName("optlist").build())
-                .addOption(builder("h").hasArg(false).desc("usage information").longOpt("help").build())
-                .addOption(builder("d").hasArg(false).desc("debug mode will print out full stack traces").longOpt("debug").build())
-                .addOption(builder("v").hasArg(false).desc("display the Groovy and JVM versions").longOpt("version").build())
-                .addOption(builder("c").argName("charset").hasArg().desc("specify the encoding of the files").longOpt("encoding").build())
-                .addOption(builder("e").argName("script").hasArg().desc("specify a command line script").build())
-                .addOption(builder("i").argName("extension").optionalArg(true).desc("modify files in place; create backup if extension is given (e.g. \'.bak\')").build())
-                .addOption(builder("n").hasArg(false).desc("process files line by line using implicit 'line' variable").build())
-                .addOption(builder("p").hasArg(false).desc("process files line by line and print result (see also -n)").build())
+                .addOption(builder("h").hasArg(false).desc("Usage information").longOpt("help").build())
+                .addOption(builder("d").hasArg(false).desc("Debug mode will print out full stack traces").longOpt("debug").build())
+                .addOption(builder("v").hasArg(false).desc("Display the Groovy and JVM versions").longOpt("version").build())
+                .addOption(builder("c").argName("charset").hasArg().desc("Specify the encoding of the files").longOpt("encoding").build())
+                .addOption(builder("e").argName("script").hasArg().desc("Specify a command line script").build())
+                .addOption(builder("i").argName("extension").optionalArg(true).desc("Modify files in place; create backup if extension is given (e.g. \'.bak\')").build())
+                .addOption(builder("n").hasArg(false).desc("Process files line by line using implicit 'line' variable").build())
+                .addOption(builder("p").hasArg(false).desc("Process files line by line and print result (see also -n)").build())
                 .addOption(builder("pa").hasArg(false).desc("Generate metadata for reflection on method parameter names (jdk8+ only)").longOpt("parameters").build())
-                .addOption(builder("l").argName("port").optionalArg(true).desc("listen on a port and process inbound lines (default: 1960)").build())
-                .addOption(builder("a").argName("splitPattern").optionalArg(true).desc("split lines using splitPattern (default '\\s') using implicit 'split' variable").longOpt("autosplit").build())
-                .addOption(builder().longOpt("indy").desc("enables compilation using invokedynamic").build())
+                .addOption(builder("l").argName("port").optionalArg(true).desc("Listen on a port and process inbound lines (default: 1960)").build())
+                .addOption(builder("a").argName("splitPattern").optionalArg(true).desc("Split lines using splitPattern (default '\\s') using implicit 'split' variable").longOpt("autosplit").build())
+                .addOption(builder().longOpt("indy").desc("Enables compilation using invokedynamic").build())
                 .addOption(builder().longOpt("configscript").hasArg().desc("A script for tweaking the configuration options").build())
                 .addOption(builder("b").longOpt("basescript").hasArg().argName("class").desc("Base class name for scripts (must derive from Script)").build());
     }
 
-    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);
-    }
-
     /**
      * Process the users request.
      *
@@ -231,10 +210,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/c4e5e225/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/c4e5e225/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 b147528..c78d25e 100644
--- a/src/main/org/codehaus/groovy/tools/GrapeMain.groovy
+++ b/src/main/org/codehaus/groovy/tools/GrapeMain.groovy
@@ -262,7 +262,7 @@ import org.apache.ivy.util.Message
 // command line parsing
 @Field Options options = new Options();
 
-options.addOption(Option.builder("D").longOpt("define").desc("define a system property").hasArg(true).argName("name=value").build());
+options.addOption(Option.builder("D").longOpt("define").desc("define a system property").numberOfArgs(2).valueSeparator().argName("name=value").build());
 options.addOption(Option.builder("r").longOpt("resolver").desc("define a grab resolver (for install)").hasArg(true).argName("url").build());
 options.addOption(Option.builder("h").hasArg(false).desc("usage information").longOpt("help").build());
 
@@ -292,10 +292,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/c4e5e225/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 fcbcec0..5e43194 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/c4e5e225/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 f99afd8..51696ad 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
@@ -30,6 +30,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.
@@ -43,8 +45,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
 {
@@ -87,7 +87,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'])
             pa(longOpt: 'parameters', messages['cli.option.parameters.description'])
         }
@@ -133,10 +133,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)
             }
         }
 
@@ -260,22 +258,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)
     }
 }
 

Reply | Threaded
Open this post in threaded view
|

[2/2] groovy git commit: GROOVY-8353: groovyConsole should support -Dname=value setting of system properties

paulk
GROOVY-8353: groovyConsole should support -Dname=value setting of system properties


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

Branch: refs/heads/GROOVY_2_6_X
Commit: 8e9b3ec2bb8154a1d69eef76ab8a3ce85cb169ba
Parents: 2974831
Author: paulk <[hidden email]>
Authored: Thu Oct 12 18:18:36 2017 +1000
Committer: paulk <[hidden email]>
Committed: Thu Oct 12 18:21:04 2017 +1000

----------------------------------------------------------------------
 .../src/main/groovy/groovy/ui/Console.groovy    | 21 ++++++++++----------
 .../main/resources/groovy/ui/Console.properties |  2 ++
 2 files changed, 12 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/8e9b3ec2/subprojects/groovy-console/src/main/groovy/groovy/ui/Console.groovy
----------------------------------------------------------------------
diff --git a/subprojects/groovy-console/src/main/groovy/groovy/ui/Console.groovy b/subprojects/groovy-console/src/main/groovy/groovy/ui/Console.groovy
index 4bfb2d4..7663719 100644
--- a/subprojects/groovy-console/src/main/groovy/groovy/ui/Console.groovy
+++ b/subprojects/groovy-console/src/main/groovy/groovy/ui/Console.groovy
@@ -67,15 +67,6 @@ import javax.swing.event.DocumentListener
  * Groovy Swing console.
  *
  * Allows user to interactively enter and execute Groovy.
- *
- * @author Danno Ferrin
- * @author Dierk Koenig, changed Layout, included Selection sensitivity, included ObjectBrowser
- * @author Alan Green more features: history, System.out capture, bind result to _
- * @author Guillaume Laforge, stacktrace hyperlinking to the current script line
- * @author Hamlet D'Arcy, AST browser
- * @author Roshan Dawrani
- * @author Paul King
- * @author Andre Steingress
  */
 class Console implements CaretListener, HyperlinkListener, ComponentListener, FocusListener {
 
@@ -212,6 +203,7 @@ class Console implements CaretListener, HyperlinkListener, ComponentListener, Fo
             V(longOpt: 'version', messages['cli.option.version.description'])
             pa(longOpt: 'parameters', messages['cli.option.parameters.description'])
             i(longOpt: 'indy', messages['cli.option.indy.description'])
+            D(longOpt: 'define', args: 2, argName: 'name=value', valueSeparator: '=', messages['cli.option.define.description'])
         }
         OptionAccessor options = cli.parse(args)
 
@@ -230,6 +222,12 @@ class Console implements CaretListener, HyperlinkListener, ComponentListener, Fo
             System.exit(0)
         }
 
+        if (options.hasOption('D')) {
+            options.getOptionProperties('D')?.each { k, v ->
+                System.setProperty(k, v)
+            }
+        }
+
         // full stack trace should not be logged to the output window - GROOVY-4663
         java.util.logging.Logger.getLogger(StackTraceUtils.STACK_LOG_NAME).useParentHandlers = false
 
@@ -246,8 +244,9 @@ class Console implements CaretListener, HyperlinkListener, ComponentListener, Fo
         def console = new Console(Console.class.classLoader?.getRootLoader(), new Binding(), baseConfig)
         console.useScriptClassLoaderForScriptExecution = true
         console.run()
-        if (args.length > 0 && !args[-1].toString().startsWith("-")) {
-            console.loadScriptFile(args[-1] as File)
+        def remaining = options.arguments()
+        if (remaining && !remaining[-1].startsWith("-")) {
+            console.loadScriptFile(remaining[-1] as File)
         }
 
     }

http://git-wip-us.apache.org/repos/asf/groovy/blob/8e9b3ec2/subprojects/groovy-console/src/main/resources/groovy/ui/Console.properties
----------------------------------------------------------------------
diff --git a/subprojects/groovy-console/src/main/resources/groovy/ui/Console.properties b/subprojects/groovy-console/src/main/resources/groovy/ui/Console.properties
index b04c0ec..3e1436b 100644
--- a/subprojects/groovy-console/src/main/resources/groovy/ui/Console.properties
+++ b/subprojects/groovy-console/src/main/resources/groovy/ui/Console.properties
@@ -28,6 +28,8 @@ cli.option.parameters.description=Generate metadata for reflection on method par
 
 cli.option.indy.description=Enable InvokeDynamic (Indy) compilation for scripts
 
+cli.option.define.description=Define a system property
+
 cli.info.version=GroovyConsole {0}
 
 # Preferences Dialog