Skip to content
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

Support quoted map keys with embedded '=' characters #594

Closed
pubudu91 opened this issue Jan 7, 2019 · 5 comments
Closed

Support quoted map keys with embedded '=' characters #594

pubudu91 opened this issue Jan 7, 2019 · 5 comments
Labels
theme: parser An issue or change related to the parser type: enhancement ✨
Milestone

Comments

@pubudu91
Copy link

pubudu91 commented Jan 7, 2019

I have the following map:

@CommandLine.Option(names = "-e")
private Map<String, String> runtimeParams = new HashMap<>();

If I give the key/val pair as follows, it splits the pair at the first "=" character it encounters.
$ <cmd> -e a=b=c=foo

Is there a way to indicate to the parser from where to split this pair? For example, I was expecting something like the following to work:
$ <cmd> -e "a=b=c"=foo

In both of the above cases, the pair is split as a and b=c=foo.

@remkop
Copy link
Owner

remkop commented Jan 7, 2019

That is a good suggestion, I like it!

(NOTE TO SELF: apply this patch)

Index: src/main/java/picocli/CommandLine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/java/picocli/CommandLine.java	(date 1546856973289)
+++ src/main/java/picocli/CommandLine.java	(date 1546856973289)
@@ -5299,7 +5299,7 @@
                 if (parser.splitQuotedStrings()) {
                     return debug(value.split(splitRegex(), limit), "Split (ignoring quotes)", value);
                 }
-                return debug(splitRespectingQuotedStrings(value, limit, parser), "Split", value);
+                return debug(splitRespectingQuotedStrings(value, limit, parser, this, splitRegex()), "Split", value);
             }
             private String[] debug(String[] result, String msg, String value) {
                 Tracer t = new Tracer();
@@ -5307,7 +5307,7 @@
                 return result;
             }
             // @since 3.7
-            private String[] splitRespectingQuotedStrings(String value, int limit, ParserSpec parser) {
+            private static String[] splitRespectingQuotedStrings(String value, int limit, ParserSpec parser, ArgSpec argSpec, String splitRegex) {
                 StringBuilder splittable = new StringBuilder();
                 StringBuilder temp = new StringBuilder();
                 StringBuilder current = splittable;
@@ -5335,22 +5335,22 @@
                     current.appendCodePoint(ch);
                 }
                 if (temp.length() > 0) {
-                    new Tracer().warn("Unbalanced quotes in [%s] for %s (value=%s)%n", temp, this, value);
+                    new Tracer().warn("Unbalanced quotes in [%s] for %s (value=%s)%n", temp, argSpec, value);
                     quotedValues.add(temp.toString());
                     temp.setLength(0);
                 }
-                String[] result = splittable.toString().split(splitRegex(), limit);
+                String[] result = splittable.toString().split(splitRegex, limit);
                 for (int i = 0; i < result.length; i++) {
                     result[i] = restoreQuotedValues(result[i], quotedValues, parser);
                 }
                 if (!quotedValues.isEmpty()) {
-                    new Tracer().warn("Unable to respect quotes while splitting value %s for %s (unprocessed remainder: %s)%n", value, this, quotedValues);
-                    return value.split(splitRegex(), limit);
+                    new Tracer().warn("Unable to respect quotes while splitting value %s for %s (unprocessed remainder: %s)%n", value, argSpec, quotedValues);
+                    return value.split(splitRegex, limit);
                 }
                 return result;
             }
 
-            private String restoreQuotedValues(String part, Queue<String> quotedValues, ParserSpec parser) {
+            private static String restoreQuotedValues(String part, Queue<String> quotedValues, ParserSpec parser) {
                 StringBuilder result = new StringBuilder();
                 boolean escaping = false, inQuote = false, skip = false;
                 for (int ch = 0, i = 0; i < part.length(); i += Character.charCount(ch)) {
@@ -7711,8 +7711,9 @@
         }
 
         private String[] splitKeyValue(ArgSpec argSpec, String value) {
-            String[] keyValue = value.split("=", 2);
-            if (keyValue.length < 2) {
+            String[] keyValue = ArgSpec.splitRespectingQuotedStrings(value, 2, config(), argSpec, "=");
+
+                if (keyValue.length < 2) {
                 String splitRegex = argSpec.splitRegex();
                 if (splitRegex.length() == 0) {
                     throw new ParameterException(CommandLine.this, "Value for option " + optionDescription("",

I did a quick experiment and with the above patch picocli should be able to support both quoted keys and quoted values. This would allow you to put as many ’=‘ characters in the keys and values as you want. I got this to work:

@Test
public void testQuotedMapKeysDefault() {
    class App {
        @Option(names = "-e")
        Map<String, String> runtimeParams = new HashMap<String, String>();
    }

    App app = new App();
    new CommandLine(app).parseArgs("-e", "\"a=b=c\"=foo");
    assertTrue(app.runtimeParams.containsKey("\"a=b=c\""));
    assertEquals("foo", app.runtimeParams.get("\"a=b=c\""));

    new CommandLine(app).parseArgs("-e", "\"a=b=c\"=\"x=y=z\"");
    assertTrue(app.runtimeParams.containsKey("\"a=b=c\""));
    assertEquals("\"x=y=z\"", app.runtimeParams.get("\"a=b=c\""));
}

This gives you a map with "a=b=c" (with quotes) as the key and "x=y=z" (with quotes) as the value. I imagine that ideally you prefer to have a=b=c and x=y=z (without the quotes) as the key/value in the map. This will be possible by setting CommandLine::setTrimQuotes(true):

@Test
public void testQuotedMapKeysTrimQuotes() {
    class App {
        @Option(names = "-e")
        Map<String, String> map = new HashMap<String, String>();
    }

    App app = new App();
    new CommandLine(app).setTrimQuotes(true).parseArgs("-e", "\"a=b=c\"=foo");
    assertTrue(app.map.containsKey("a=b=c"));
    assertEquals("foo", app.map.get("a=b=c"));

    new CommandLine(app).setTrimQuotes(true).parseArgs("-e", "\"a=b=c\"=x=y=z");
    assertTrue(app.map.keySet().toString(), app.map.containsKey("a=b=c"));
    assertEquals("x=y=z", app.map.get("a=b=c"));
}

One potential drawback: if CommandLine::setTrimQuotes() is set to true, and a parameter is considered “quoted” because it starts and ends with a " quote character, picocli will first trim the leading and trailing " quotes from the parameter before processing it further. So if both key and value are quoted (perhaps because the key contains '=' characters and the value contains ' ' space characters), you will need to enclose that whole string in another set of quotes. For example:

@Test
public void testQuotedMapKeysAndValuesBothHaveQuotes() {
    class App {
        @Option(names = "-e")
        Map<String, String> map = new HashMap<String, String>();
    }

    App app = new App();
    new CommandLine(app).setTrimQuotes(true).parseArgs("-e", "\"\"a=b=c\"=\"x y z\"\"");
    assertTrue(app.map.keySet().toString(), app.map.containsKey("a=b=c"));
    assertEquals("x y z", app.map.get("a=b=c"));
}

@remkop remkop added this to the 3.9.1 milestone Jan 7, 2019
@remkop remkop changed the title The parser splits the key/value pair from the first "=" character it encounters Support quoted map keys with embedded '=' characters Jan 7, 2019
@remkop remkop closed this as completed in b19a4e5 Jan 7, 2019
@remkop
Copy link
Owner

remkop commented Jan 7, 2019

This has been pushed to master. Can you verify?

@remkop
Copy link
Owner

remkop commented Jan 7, 2019

Note: During testing I found some edge cases when the option has a split regex:

class App {
  @Option(names = "--option", split = ",")
  Map<String, String> map;
}

Some examples:

# gives this map { "k1": "v1", "k2": "v2" }
--option=k1=v1,k2=v2

# gives { "k1": "v1 w1", "k2": "v2 w2" }
--option="k1=v1 w1","k2=v2 w2"

# not yet supported (needs #595)
--option="'k1=key'=v1 w1","'k2=key'=v2 w2"

It is not yet possible to combine quoted keys (with embedded '=') with quoted key-value pairs.

I raised #595 to support nested quoting in the future.

@pubudu91
Copy link
Author

pubudu91 commented Jan 8, 2019

Thanks for the prompt response and solution! I tested it and it's working. :D I need the key with the quotes, so no worries with the problems with trimming.

@remkop
Copy link
Owner

remkop commented Jan 8, 2019

Thanks for the confirmation!

@remkop remkop added the theme: parser An issue or change related to the parser label Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: parser An issue or change related to the parser type: enhancement ✨
Projects
None yet
Development

No branches or pull requests

2 participants