From 1024d2df1ea7fc0e2aec3d5586e94535240d436b Mon Sep 17 00:00:00 2001 From: Brent Yorgey Date: Mon, 4 Sep 2023 22:44:02 -0500 Subject: [PATCH] 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 --- data/scenarios/Tutorials/type-errors.yaml | 2 +- data/scenarios/Tutorials/types.yaml | 3 ++- src/Swarm/TUI/Controller.hs | 31 +++++++++++++---------- src/Swarm/TUI/Model/Repl.hs | 31 +++++++++++++---------- src/Swarm/TUI/Model/UI.hs | 7 ----- src/Swarm/TUI/View.hs | 12 +++------ 6 files changed, 42 insertions(+), 44 deletions(-) diff --git a/data/scenarios/Tutorials/type-errors.yaml b/data/scenarios/Tutorials/type-errors.yaml index 205b77f56..2c9afb66d 100644 --- 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 index ef9057979..e7b81cf42 100644 --- 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 index 6ddf512d1..10af94924 100644 --- 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 index 474103f10..594b4ae97 100644 --- 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 index 3a2fb2ec7..2abf24e88 100644 --- 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 index a1c30e3d4..bfea957f7 100644 --- 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] ------------------------------------------------------------