Print REPL errors inline and get rid of error popup (#1487)

Closes #1461. Errors which used to be displayed in a pop-up window (parse errors, type errors) are now displayed inline in the REPL window instead. - Get rid of the pop-up error dialog - Also fix a bug introduced in #1481 where the REPL history would not be properly cleared when first starting a new scenario, because the old cache was still being used
swarm-game · Sep 5, 2023 · 1024d2d · 1024d2d
1 parent 15dd824
commit 1024d2d
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 44 deletions.
diff --git a/data/scenarios/Tutorials/type-errors.yaml b/data/scenarios/Tutorials/type-errors.yaml
@@ -8,7 +8,7 @@ objectives:
         Let's see what happens when you enter something that does not type check.
         Try typing `turn 1`{=snippet} at the REPL prompt.  Clearly this is nonsense, and
         the expression will be highlighted in red.  To see what the error is, hit **Enter**.
-        A box will pop up with a type (or parser) error.
+        The REPL will print out a type error.
       - "Some other type errors for you to try:"
       - |
         `turn move`{=snippet}

diff --git a/data/scenarios/Tutorials/types.yaml b/data/scenarios/Tutorials/types.yaml
@@ -12,7 +12,8 @@ objectives:
         its type will be displayed in gray text at the top right of the window.
       - For example, if you try typing `move`, you can see that it has
         type `cmd unit`{=type}, which means that `move` is a command which
-        returns a value of the `unit`{=type} type (also written `()`).
+        returns a value of the `unit`{=type} type (the only value of
+        type `unit`{=type} is called `()`).
       - As another example, you can see that `turn` has type `dir -> cmd unit`{=type},
         meaning that `turn` is a function which takes a direction as input
         and results in a command.

diff --git a/src/Swarm/TUI/Controller.hs b/src/Swarm/TUI/Controller.hs
@@ -253,7 +253,7 @@ handleNewGameMenuEvent scenarioStack@(curMenu :| rest) = \case
   Key V.KEnter ->
     case snd <$> BL.listSelectedElement curMenu of
       Nothing -> continueWithoutRedraw
-      Just (SISingle siPair) -> startGame siPair Nothing
+      Just (SISingle siPair) -> invalidateCache >> startGame siPair Nothing
       Just (SICollection _ c) -> do
         cheat <- use $ uiState . uiCheatMode
         uiState . uiMenu .= NewGameMenu (NE.cons (mkScenarioList cheat c) scenarioStack)
@@ -305,9 +305,8 @@ handleMainEvent ev = do
         WinConditions (Won _) _ -> toggleModal $ ScenarioEndModal WinModal
         WinConditions (Unwinnable _) _ -> toggleModal $ ScenarioEndModal LoseModal
         _ -> toggleModal QuitModal
-    VtyEvent (V.EvResize _ _) -> invalidateCacheEntry WorldCache
+    VtyEvent (V.EvResize _ _) -> invalidateCache
     Key V.KEsc
-      | isJust (s ^. uiState . uiError) -> uiState . uiError .= Nothing
       | Just m <- s ^. uiState . uiModal -> do
           safeAutoUnpause
           uiState . uiModal .= Nothing
@@ -472,8 +471,13 @@ handleModalEvent = \case
     case dialogSelection =<< mdialog of
       Just (Button QuitButton, _) -> quitGame
       Just (Button KeepPlayingButton, _) -> toggleModal KeepPlayingModal
-      Just (Button StartOverButton, StartOver currentSeed siPair) -> restartGame currentSeed siPair
-      Just (Button NextButton, Next siPair) -> quitGame >> startGame siPair Nothing
+      Just (Button StartOverButton, StartOver currentSeed siPair) -> do
+        invalidateCache
+        restartGame currentSeed siPair
+      Just (Button NextButton, Next siPair) -> do
+        quitGame
+        invalidateCache
+        startGame siPair Nothing
       _ -> return ()
   ev -> do
     Brick.zoom (uiState . uiModal . _Just . modalDialog) (handleDialogEvent ev)
@@ -964,14 +968,12 @@ stripCmd pty = pty
 -- REPL events
 ------------------------------------------------------------
 
--- | Set the REPLForm to the given value, resetting type error checks to Nothing
---   and removing uiError.
+-- | Set the REPL to the given text and REPL prompt type.
 resetREPL :: T.Text -> REPLPrompt -> UIState -> UIState
 resetREPL t r ui =
   ui
     & uiREPL . replPromptText .~ t
     & uiREPL . replPromptType .~ r
-    & uiError .~ Nothing
 
 -- | Handle a user input event for the REPL.
 handleREPLEvent :: BrickEvent Name AppEvent -> EventM Name AppState ()
@@ -997,7 +999,10 @@ handleREPLEvent x = do
           _ ->
             if T.null uinput
               then uiState . uiREPL . replControlMode .= Piloting
-              else uiState . uiError ?= "Please clear the REPL first."
+              else do
+                let err = REPLError "Please clear the REPL before engaging pilot mode."
+                uiState . uiREPL . replHistory %= addREPLItem err
+                invalidateCacheEntry REPLHistoryCache
     MetaChar 'k' -> do
       when (isJust (s ^. gameState . inputHandler)) $ do
         curMode <- use $ uiState . uiREPL . replControlMode
@@ -1074,15 +1079,15 @@ runBaseWebCode uinput = do
     runBaseCode topCtx uinput
 
 runBaseCode :: (MonadState AppState m) => RobotContext -> T.Text -> m ()
-runBaseCode topCtx uinput =
+runBaseCode topCtx uinput = do
+  uiState . uiREPL . replHistory %= addREPLItem (REPLEntry uinput)
+  uiState %= resetREPL "" (CmdPrompt [])
   case processTerm' (topCtx ^. defTypes) (topCtx ^. defReqs) uinput of
     Right mt -> do
-      uiState %= resetREPL "" (CmdPrompt [])
-      uiState . uiREPL . replHistory %= addREPLItem (REPLEntry uinput)
       uiState . uiREPL . replHistory . replHasExecutedManualInput .= True
       runBaseTerm topCtx mt
     Left err -> do
-      uiState . uiError ?= err
+      uiState . uiREPL . replHistory %= addREPLItem (REPLError err)
 
 runBaseTerm :: (MonadState AppState m) => RobotContext -> Maybe ProcessedTerm -> m ()
 runBaseTerm topCtx =

diff --git a/src/Swarm/TUI/Model/Repl.hs b/src/Swarm/TUI/Model/Repl.hs
@@ -78,6 +78,8 @@ data REPLHistItem
     REPLEntry Text
   | -- | A response printed by the system.
     REPLOutput Text
+  | -- | An error printed by the system.
+    REPLError Text
   deriving (Eq, Ord, Show, Read)
 
 instance ToSample REPLHistItem where
@@ -87,6 +89,7 @@ instance ToJSON REPLHistItem where
   toJSON e = case e of
     REPLEntry x -> object ["in" .= x]
     REPLOutput x -> object ["out" .= x]
+    REPLError x -> object ["err" .= x]
 
 -- | Useful helper function to only get user input text.
 getREPLEntry :: REPLHistItem -> Maybe Text
@@ -103,6 +106,7 @@ replItemText :: REPLHistItem -> Text
 replItemText = \case
   REPLEntry t -> t
   REPLOutput t -> t
+  REPLError t -> t
 
 -- | History of the REPL with indices (0 is first entry) to the current
 --   line and to the first entry since loading saved history.
@@ -130,23 +134,24 @@ replIndex :: Lens' REPLHistory Int
 -- It will be set on load and reset on save (happens during exit).
 replStart :: Lens' REPLHistory Int
 
--- | Note: Instead of adding a dedicated field to the REPLHistory record,
--- an early attempt entailed checking for:
+-- | Keep track of whether the user has explicitly executed commands
+--   at the REPL prompt, thus making them ineligible for code size scoring.
 --
---    _replIndex > _replStart
+--   Note: Instead of adding a dedicated field to the REPLHistory record,
+--   an early attempt entailed checking for:
 --
--- However, executing an initial script causes
--- a "REPLOutput" to be appended to the REPL history,
--- which increments the replIndex, and thus makes
--- the Index greater than the Start even though
--- the player has input not commands into the REPL.
+--     _replIndex > _replStart
 --
--- Therefore, a dedicated boolean is introduced into
--- REPLHistory which simply latches True when the user
--- has input a command.
+--   However, executing an initial script causes a "REPLOutput" to be
+--   appended to the REPL history, which increments the replIndex, and
+--   thus makes the Index greater than the Start even though the
+--   player has not input commands directly into the REPL.
 --
--- An alternative is described here:
--- https://github.com/swarm-game/swarm/pull/974#discussion_r1112380380
+--   Therefore, a dedicated boolean is introduced into REPLHistory
+--   which simply latches True when the user has input a command.
+--
+--   An alternative is described here:
+--   https://github.com/swarm-game/swarm/pull/974#discussion_r1112380380
 replHasExecutedManualInput :: Lens' REPLHistory Bool
 
 -- | Create new REPL history (i.e. from loaded history file lines).

diff --git a/src/Swarm/TUI/Model/UI.hs b/src/Swarm/TUI/Model/UI.hs
@@ -21,7 +21,6 @@ module Swarm.TUI.Model.UI (
   uiInventorySort,
   uiInventorySearch,
   uiScrollToEnd,
-  uiError,
   uiModal,
   uiGoal,
   uiHideGoals,
@@ -106,7 +105,6 @@ data UIState = UIState
   , _uiInventorySort :: InventorySortOptions
   , _uiInventorySearch :: Maybe Text
   , _uiScrollToEnd :: Bool
-  , _uiError :: Maybe Text
   , _uiModal :: Maybe Modal
   , _uiGoal :: GoalDisplay
   , _uiHideGoals :: Bool
@@ -177,10 +175,6 @@ uiInventory :: Lens' UIState (Maybe (Int, BL.List Name InventoryListEntry))
 --   (used when a new log message is appended).
 uiScrollToEnd :: Lens' UIState Bool
 
--- | When this is @Just@, it represents a popup box containing an
---   error message that is shown on top of the rest of the UI.
-uiError :: Lens' UIState (Maybe Text)
-
 -- | When this is @Just@, it represents a modal to be displayed on
 --   top of the UI, e.g. for the Help screen.
 uiModal :: Lens' UIState (Maybe Modal)
@@ -320,7 +314,6 @@ initUIState speedFactor showMainMenu cheatMode = do
           , _uiInventorySort = defaultSortOptions
           , _uiInventorySearch = Nothing
           , _uiScrollToEnd = False
-          , _uiError = Nothing
           , _uiModal = Nothing
           , _uiGoal = emptyGoalDisplay
           , _uiHideGoals = False

diff --git a/src/Swarm/TUI/View.hs b/src/Swarm/TUI/View.hs
@@ -561,20 +561,13 @@ chooseCursor s locs = case s ^. uiState . uiModal of
   Nothing -> showFirstCursor s locs
   Just _ -> Nothing
 
--- | Render the error dialog window with a given error message
-renderErrorDialog :: Text -> Widget Name
-renderErrorDialog err = renderDialog (dialog (Just $ str "Error") Nothing (maxModalWindowWidth `min` requiredWidth)) errContent
- where
-  errContent = txtWrapWith indent2 {preserveIndentation = True} err
-  requiredWidth = 2 + maximum (textWidth <$> T.lines err)
-
--- | Draw the error dialog window, if it should be displayed right now.
+-- | Draw a dialog window, if one should be displayed right now.
 drawDialog :: AppState -> Widget Name
 drawDialog s = case s ^. uiState . uiModal of
   Just (Modal mt d) -> renderDialog d $ case mt of
     GoalModal -> drawModal s mt
     _ -> maybeScroll ModalViewport $ drawModal s mt
-  Nothing -> maybe emptyWidget renderErrorDialog (s ^. uiState . uiError)
+  Nothing -> emptyWidget
 
 -- | Draw one of the various types of modal dialog.
 drawModal :: AppState -> ModalType -> Widget Name
@@ -1350,6 +1343,7 @@ drawREPL s =
   base = s ^. gameState . robotMap . at 0
   fmt (REPLEntry e) = txt $ "> " <> e
   fmt (REPLOutput t) = txt t
+  fmt (REPLError t) = txtWrapWith indent2 {preserveIndentation = True} t
   mayDebug = [drawRobotMachine s True | s ^. uiState . uiShowDebug]
 
 ------------------------------------------------------------