Allow controlling navigator startup in daml.yaml (#5155)

* Allow controlling navigator startup in daml.yaml

While I’m not entirely convinced the default atm is right, making it
configurable seems like an easier solution than bikeshedding the default.

changelog_begin

- [DAML Assistant] You can now disable starting navigator as part of
  ``daml start`` in your ``daml.yaml`` file by adding ``start-navigator: false``.

changelog_end

* Update docs/source/tools/assistant.rst

Co-Authored-By: Martin Huschenbett <martin.huschenbett@posteo.me>

* Use --start-navigator=true in the docs

Co-authored-by: Martin Huschenbett <martin.huschenbett@posteo.me>

daml-assistant/daml-helper/src/DA/Daml/Helper/Main.hs

@@ -42,7 +42,7 @@ data Command
     | Start
       { sandboxPortM :: Maybe SandboxPort
       , openBrowser :: OpenBrowser
-      , startNavigator :: StartNavigator
+      , startNavigator :: Maybe StartNavigator
       , jsonApiCfg :: JsonApiConfig
       , onStartM :: Maybe String
       , waitForSignal :: WaitForSignal
@@ -112,7 +112,7 @@ commandParser = subparser $ fold
     startCmd = Start
         <$> optional (SandboxPort <$> option auto (long "sandbox-port" <> metavar "PORT_NUM" <>     help "Port number for the sandbox"))
         <*> (OpenBrowser <$> flagYesNoAuto "open-browser" True "Open the browser after navigator" idm)
-        <*> (StartNavigator <$> flagYesNoAuto "start-navigator" True "Start navigator after sandbox" idm)
+        <*> optional navigatorFlag
         <*> jsonApiCfg
         <*> optional (option str (long "on-start" <> metavar "COMMAND" <> help "Command to run once sandbox and navigator are running."))
         <*> (WaitForSignal <$> flagYesNoAuto "wait-for-signal" True "Wait for Ctrl+C or interrupt after starting servers." idm)
@@ -122,6 +122,22 @@ commandParser = subparser $ fold
         <*> (ScriptOptions <$> many (strOption (long "script-option" <> metavar "SCRIPT_OPTION" <> help "Pass option to DAML script interpreter")))
         <*> stdinCloseOpt
 
+    navigatorFlag =
+        -- We do not use flagYesNoAuto here since that doesn’t allow us to differentiate
+        -- if the flag was passed explicitly or not.
+        StartNavigator <$>
+        option reader (long "start-navigator" <> help helpText <> completeWith ["true", "false"] <> idm)
+        where
+            reader = eitherReader $ \case
+                -- We allow for both yes and true since we want a boolean in daml.yaml
+                "true" -> Right True
+                "yes" -> Right True
+                "false" -> Right False
+                "no" -> Right False
+                "auto" -> Right True
+                s -> Left ("Expected \"yes\", \"true\", \"no\", \"false\" or \"auto\" but got " <> show s)
+            -- To make things less confusing, we do not mention yes, no and auto here.
+            helpText = "Start navigator as part of daml start. Can be set to true or false. Defaults to true."
     deployCmdInfo = mconcat
         [ progDesc $ concat
               [ "Deploy the current DAML project to a remote DAML ledger. "

daml-assistant/daml-helper/src/DA/Daml/Helper/Run.hs

@@ -766,7 +766,7 @@ withOptsFromProjectConfig fieldName cliOpts projectConfig = do
 
 runStart
     :: Maybe SandboxPort
-    -> StartNavigator
+    -> Maybe StartNavigator
     -> JsonApiConfig
     -> OpenBrowser
     -> Maybe String
@@ -778,7 +778,7 @@ runStart
     -> IO ()
 runStart
   sandboxPortM
-  (StartNavigator shouldStartNavigator)
+  mbStartNavigator
   (JsonApiConfig mbJsonApiPort)
   (OpenBrowser shouldOpenBrowser)
   onStartM
@@ -797,6 +797,13 @@ runStart
     mbInitScript :: Maybe String <-
         requiredE "Failed to parse init-script" $
         queryProjectConfig ["init-script"] projectConfig
+    shouldStartNavigator :: Bool <- case mbStartNavigator of
+        -- If an option is passed explicitly, we use it, otherwise we read daml.yaml.
+        Nothing ->
+            fmap (fromMaybe True) $
+            requiredE "Failed to parse start-navigator" $
+            queryProjectConfig ["start-navigator"] projectConfig
+        Just (StartNavigator explicit) -> pure explicit
     sandboxOpts <- withOptsFromProjectConfig "sandbox-options" sandboxOpts projectConfig
     navigatorOpts <- withOptsFromProjectConfig "navigator-options" navigatorOpts projectConfig
     jsonApiOpts <- withOptsFromProjectConfig "json-api-options" jsonApiOpts projectConfig
@@ -804,7 +811,7 @@ runStart
     doBuild
     let scenarioArgs = maybe [] (\scenario -> ["--scenario", scenario]) mbScenario
     withSandbox sandboxPort (darPath : scenarioArgs ++ sandboxOpts) $ \sandboxPh -> do
-        withNavigator' sandboxPh sandboxPort navigatorPort navigatorOpts $ \navigatorPh -> do
+        withNavigator' shouldStartNavigator sandboxPh sandboxPort navigatorPort navigatorOpts $ \navigatorPh -> do
             whenJust mbInitScript $ \initScript -> do
                 procScript <- toAssistantCommand $
                     [ "script"
@@ -831,7 +838,7 @@ runStart
     where
         navigatorPort = NavigatorPort 7500
         defaultSandboxPort = SandboxPort 6865
-        withNavigator' sandboxPh =
+        withNavigator' shouldStartNavigator sandboxPh =
             if shouldStartNavigator
                 then withNavigator
                 else (\_ _ _ f -> f sandboxPh)

docs/source/tools/assistant.rst

@@ -138,6 +138,10 @@ Here is what each field means:
 - ``json-api-options``: a list of options that will be passed to the HTTP JSON API in ``daml start``.
 - ``script-options``: a list of options that will be passed to the DAML script
   runner when running the ``init-script`` as part of ``daml start``.
+- ``start-navigator``: Controls whether navigator is started as part
+  of ``daml start``. Defaults to ``true``. If this is specified as a CLI argument,
+  say ``daml start --start-navigator=true``, the CLI argument takes precedence over
+  the value in ``daml.yaml``.
 
 ..  TODO (@robin-da) document the dependency syntax