[#1572] Remove redundant braces in ArgGroup synopsis

Closes #1572
remkop · Feb 14, 2022 · 20b3cda · 20b3cda
1 parent 0043b39
commit 20b3cda
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 9 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -51,6 +51,7 @@ class MyIntConverter implements ITypeConverter<Integer> {
 * [#1380][#1505] API, bugfix: `requiredOptionMarker` should not be displayed on `ArgGroup` options. Thanks to [Ahmed El Khalifa](https://github.com/ahmede41) for the pull request.
 * [#1563] API: Add constructor to `PicocliSpringFactory` to allow custom fallback `IFactory`. Thanks to [Andrew Holland](https://github.com/a1dutch) for raising this.
 * [#1571] Enhancement: Variables in values from the default value provider should be interpolated. Thanks to [Bas Passon](https://github.com/bpasson) for raising this.
+* [#1572] Enhancement: Remove redundant braces in ArgGroup synopsis.
 * [#1573] DEP: Bump JLine3 version to 3.21.0 from 3.19.0.
 
 ## <a name="4.7.0-deprecated"></a> Deprecations

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -10411,8 +10411,10 @@ public Text synopsisText(Help.ColorScheme colorScheme, Set<ArgSpec> outparam_gro
             }
 
             private Text synopsisUnitText(Help.ColorScheme colorScheme, Text synopsis) {
-                String prefix = multiplicity().min() > 0 ? "(" : "[";
-                String postfix = multiplicity().min() > 0 ? ")" : "]";
+                String requiredOpen = args().size() > 1 || !subgroups().isEmpty() ? "(" : "";
+                String requiredClose = args().size() > 1 || !subgroups().isEmpty() ? ")" : "";
+                String prefix = multiplicity().min() > 0 ? requiredOpen : "[";
+                String postfix = multiplicity().min() > 0 ? requiredClose : "]";
                 return colorScheme.text(prefix).concat(synopsis).concat(postfix);
             }
 

diff --git a/src/test/java/picocli/ArgGroupTest.java b/src/test/java/picocli/ArgGroupTest.java
@@ -253,7 +253,7 @@ public void testAnOptionCannotBeInMultipleGroups() {
             spec.addArgGroup(group);
             fail("Expected exception");
         } catch (CommandLine.DuplicateNameException ex) {
-            assertEquals("An option cannot be in multiple groups but -x is in (-x) and [-x]. " +
+            assertEquals("An option cannot be in multiple groups but -x is in -x and [-x]. " +
                     "Refactor to avoid this. For example, (-a | (-a -b)) can be rewritten " +
                     "as (-a [-b]), and (-a -b | -a -c) can be rewritten as (-a (-b | -c)).", ex.getMessage());
         }
@@ -1776,7 +1776,7 @@ public void testRepeatingGroupsValidation() {
             cmd.parseArgs("-a", "1");
             fail("Expected exception");
         } catch (CommandLine.ParameterException ex) {
-            assertEquals("Error: Group: (-a=<a>) must be specified 2 times but was matched 1 times", ex.getMessage());
+            assertEquals("Error: Group: -a=<a> must be specified 2 times but was matched 1 times", ex.getMessage());
         }
     }
 
@@ -2370,8 +2370,8 @@ public void run() {
     @Test
     public void testIssue722() {
         String expected = String.format("" +
-                "create --level-0 <l0> (--level-1 <l1> (--level-2a <l2a>) (--level-2b <l2b>%n" +
-                "       (--level-3a <l3a>) (--level-3b <l3b>)))%n");
+                "create --level-0 <l0> (--level-1 <l1> --level-2a <l2a> (--level-2b <l2b>%n" +
+                "       --level-3a <l3a> --level-3b <l3b>))%n");
 
         CommandLine cmd = new CommandLine(new Issue722.CreateCommand());
         Help help = new Help(cmd.getCommandSpec(), Help.defaultColorScheme(Help.Ansi.OFF));
@@ -4134,8 +4134,8 @@ public void run() {
 
         }
     }
-    
-    
+
+
     // String literals for Issue 1409
     final String sampleX = "ANOTHER VALUE";
     final String errorX = "Default value for X incorrect";
@@ -4313,4 +4313,4 @@ public void testIssue1409ModInitializeGroup2() {
         assertEquals(errorB2,"z", obj.optXAndGroupOneOrGroupTwo.oneORtwo.two.b2);
     }
 
-}
+}