Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion data/scenarios/Tutorials/type-errors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
3 changes: 2 additions & 1 deletion data/scenarios/Tutorials/types.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
31 changes: 18 additions & 13 deletions src/Swarm/TUI/Controller.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 ()
Expand All @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
31 changes: 18 additions & 13 deletions src/Swarm/TUI/Model/Repl.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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).
Expand Down
7 changes: 0 additions & 7 deletions src/Swarm/TUI/Model/UI.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ module Swarm.TUI.Model.UI (
uiInventorySort,
uiInventorySearch,
uiScrollToEnd,
uiError,
uiModal,
uiGoal,
uiHideGoals,
Expand Down Expand Up @@ -106,7 +105,6 @@ data UIState = UIState
, _uiInventorySort :: InventorySortOptions
, _uiInventorySearch :: Maybe Text
, _uiScrollToEnd :: Bool
, _uiError :: Maybe Text
, _uiModal :: Maybe Modal
, _uiGoal :: GoalDisplay
, _uiHideGoals :: Bool
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -320,7 +314,6 @@ initUIState speedFactor showMainMenu cheatMode = do
, _uiInventorySort = defaultSortOptions
, _uiInventorySearch = Nothing
, _uiScrollToEnd = False
, _uiError = Nothing
, _uiModal = Nothing
, _uiGoal = emptyGoalDisplay
, _uiHideGoals = False
Expand Down
12 changes: 3 additions & 9 deletions src/Swarm/TUI/View.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]

------------------------------------------------------------
Expand Down
Loading