Code review fixes

Options/Applicative/Common.hs

@@ -279,9 +279,19 @@ treeMapParser g = simplify . go False False False g
       where r' = r || hasArg p1
     go m d r f p@(AltP p1 p2) =
       AltNode altNodeType [go m d' r f p1, go m d' r f p2]
-      where altNodeType | has_default p && not (has_default p1 && has_default p2) = AltDefault
-                        | otherwise = AltNoDefault
-            d' = d || altNodeType == AltDefault
+      where
+        -- The 'altNodeType' variable tracks whether or not this node of the
+        -- 'OptTree' ought to be displayed with brackets or not. Generally, we want
+        -- to put brackets around it when the parser has optional arguments, but if
+        -- *both* of its children also have optional arguments, then we don't put
+        -- brackets around the top-level because that would be redundant.
+        altNodeType | has_default p && not (has_default p1 && has_default p2) = AltDefault
+                    | otherwise = AltNoDefault
+        -- The 'd' variable tracks whether the option nodes at the leaves have
+        -- optional arguments so that when we want to hide optional arguments, we
+        -- have the information needed to do that. An option can be considered
+        -- optional in this sense if any of its parents were optional.
+        d' = d || altNodeType == AltDefault
     go _ d r f (BindP p k) =
       let go' = go True d r f p
       in case evalParser p of

Options/Applicative/Help/Core.hs

@@ -57,15 +57,10 @@ optDesc pprefs style info opt =
       render chunk
         | not show_opt
         = mempty
-        | isEmpty chunk || not (descSurround style)
-        = mappend chunk suffix
-        --  | hinfoDefault info
-        -- = mappend chunk suffix
-        | null (drop 1 descs)
-        = mappend chunk suffix
+        | not (hinfoDefault info) && length descs > 1
+        = fmap parens chunk <> suffix
         | otherwise
-        = mappend chunk suffix
---         = mappend (fmap parens chunk) suffix
+        = chunk <> suffix
   in maybe id fmap (optDescMod opt) (render desc')
 
 -- | Generate descriptions for commands.
@@ -104,7 +99,7 @@ bracket :: Bool -> Chunk Doc -> Chunk Doc
 bracket b chunk = if b then fmap brackets chunk else chunk
 
 fold_tree :: OptTree (Chunk Doc) -> Chunk Doc
-fold_tree (Leaf x) = x -- bracket b x
+fold_tree (Leaf x) = x
 fold_tree (MultNode xs) = foldr ((<</>>) . fold_tree) mempty xs
 fold_tree (AltNode b xs) = bracket (b == AltDefault)
                            . alt_node