CompletionItemOptions: make mempty minimal, add legacyCompletionItemO…

…ptions

src/Options/Applicative/BashCompletion.hs

@@ -120,12 +120,12 @@ bashCompletionQuery pinfo pprefs features ws i _ = case runCompletion compl ppre
     opt_completions argPolicy reachability opt = case optMain opt of
       OptReader ns _ _
          | argPolicy /= AllPositionals
-        -> return . fmap defaultCompletionItem . add_opt_help opt $ show_names ns
+        -> return . fmap legacyCompletionItem . add_opt_help opt $ show_names ns
          | otherwise
         -> return []
       FlagReader ns _
          | argPolicy /= AllPositionals
-        -> return . fmap defaultCompletionItem . add_opt_help opt $ show_names ns
+        -> return . fmap legacyCompletionItem . add_opt_help opt $ show_names ns
          | otherwise
         -> return []
       ArgReader rdr
@@ -137,7 +137,7 @@ bashCompletionQuery pinfo pprefs features ws i _ = case runCompletion compl ppre
          | argumentIsUnreachable reachability
         -> return []
          | otherwise
-        -> return . fmap defaultCompletionItem . with_cmd_help $ filter (is_completion . fst) ns
+        -> return . fmap legacyCompletionItem . with_cmd_help $ filter (is_completion . fst) ns
 
     -- When doing enriched completions, add any help specified
     -- to the completion variables (tab separated).

src/Options/Applicative/Types.hs

@@ -21,11 +21,12 @@ module Options.Applicative.Types (
   Parser(..),
   ParserM(..),
   Completer(..),
-  mkCompleter,
   mkCompleterWithOptions,
+  mkCompleter,
   CompletionItem(..),
-  defaultCompletionItem,
+  legacyCompletionItem,
   CompletionItemOptions(..),
+  legacyCompletionItemOptions,
   CompletionResult(..),
   ParserFailure(..),
   ParserResult(..),
@@ -314,43 +315,64 @@ data CompletionItem = CompletionItem {
   ciOptions :: CompletionItemOptions,
   ciValue :: String
 }
-defaultCompletionItem :: String -> CompletionItem
-defaultCompletionItem = CompletionItem mempty
+-- | A set of defaults that includes the bells and whistles that
+-- were previously added by the shell.
+--
+-- For the minimal shell behavior, use @'CompletionItem' mempty@
+--
+-- This adds spaces to unambiguous completions (@'cioAddSpace' = True@) and
+-- treats the completions as files (@'cioFiles' = True@).
+legacyCompletionItem :: String -> CompletionItem
+legacyCompletionItem = CompletionItem CompletionItemOptions { cioAddSpace = True, cioFiles = True }
 
 data CompletionItemOptions = CompletionItemOptions {
-  -- | Whether to add a space after the completion. Defaults to 'True'.
+  -- | Whether to add a space after the completion.
   --
   -- Set this value to 'False' if the completion is only a prefix of the final
   -- valid values.
+  --
+  -- 'mempty': 'False'.
+  --
+  -- 'legacyCompletionItemOptions': 'True'.
+  --
   cioAddSpace :: Bool,
 
   -- | Whether to treat the completions as file names (if they exists) and
   -- add a trailing slash to completions that are directories.
-  -- Defaults to 'True'
+  --
+  -- 'mempty': 'False'.
+  --
+  -- 'legacyCompletionItemOptions': 'True'.
+  --
   cioFiles :: Bool
 }
+-- | Combines field-wise. Uses '||' for fields that have 'False' for 'mempty'.
 instance Semigroup CompletionItemOptions where
   a <> b =
     CompletionItemOptions {
-      cioAddSpace = cioAddSpace a && cioAddSpace b,
-      cioFiles = cioFiles a && cioFiles b
+      cioAddSpace = cioAddSpace a || cioAddSpace b,
+      cioFiles = cioFiles a || cioFiles b
     }
+-- | 'mempty' is minimal. See per-field docs.
 instance Monoid CompletionItemOptions where
-  mempty = CompletionItemOptions True True
+  mempty = CompletionItemOptions False False
   mappend = (<>)
 
+legacyCompletionItemOptions :: CompletionItemOptions
+legacyCompletionItemOptions = CompletionItemOptions { cioAddSpace = True, cioFiles = True }
+
 -- | A shell complete function.
 newtype Completer = Completer
   { runCompleter :: String -> IO [CompletionItem] }
 
--- | Smart constructor for a 'Completer'
-mkCompleter :: (String -> IO [String]) -> Completer
-mkCompleter f = Completer (fmap (map (CompletionItem mempty)) . f)
-
 -- | Smart constructor for a 'Completer'
 mkCompleterWithOptions :: (String -> IO [CompletionItem]) -> Completer
 mkCompleterWithOptions = Completer
 
+-- | Smart constructor for a 'Completer' via 'legacyCompletionItem'.
+mkCompleter :: (String -> IO [String]) -> Completer
+mkCompleter f = Completer (fmap (map legacyCompletionItem) . f)
+
 instance Semigroup Completer where
   (Completer c1) <> (Completer c2) =
     Completer $ \s -> (++) <$> c1 s <*> c2 s

tests/test.hs

@@ -468,10 +468,10 @@ prop_completion_rich_lengths = once . ioProperty $
     Failure _   -> return $ counterexample "unexpected failure" failed
     Success val -> return $ counterexample ("unexpected result " ++ show val) failed
 
-prop_completion_v1_default :: Property
-prop_completion_v1_default = once . ioProperty $
+prop_completion_v1_legacy :: Property
+prop_completion_v1_legacy = once . ioProperty $
   let p :: Parser String
-      p = strArgument (completer (mkCompleterWithOptions (pure (pure [CompletionItem mempty "reachable"]))))
+      p = strArgument (completer (mkCompleterWithOptions (pure (pure [legacyCompletionItem "reachable"]))))
       i = info p idm
       result = run i [ "--optparse-completion-version", "1"
                      , "--bash-completion-index=0"
@@ -486,7 +486,7 @@ prop_completion_v1_default = once . ioProperty $
 prop_completion_v1_minimal :: Property
 prop_completion_v1_minimal = once . ioProperty $
   let p :: Parser String
-      p = strArgument (completer (mkCompleterWithOptions (pure (pure [CompletionItem (mempty { cioAddSpace = False, cioFiles = False }) "reachable"]))))
+      p = strArgument (completer (mkCompleterWithOptions (pure (pure [CompletionItem mempty "reachable"]))))
       i = info p idm
       result = run i [ "--optparse-completion-version", "1"
                      , "--bash-completion-index=0"