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

Improvements to scrolling #1481

Merged
merged 8 commits into from
Sep 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
cache REPL history
I think this is the proper way to do things: we give the entire REPL
history to brick so that it can render the entire thing and thus draw
the scroll bar correctly, scroll through the history with PgUp/PgDown
etc., but we cache it so that it is only re-rendered when the history
changes (i.e. when a new input is entered, or a new output becomes
available).
  • Loading branch information
byorgey committed Sep 3, 2023
commit 0b08a3b702d1734cbc7c9257e61b2cda90818049
5 changes: 4 additions & 1 deletion src/Swarm/TUI/Controller.hs
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,7 @@ updateUI = do
let itName = fromString $ "it" ++ show itIx
let out = T.intercalate " " [itName, ":", prettyText finalType, "=", into (prettyValue v)]
uiState . uiREPL . replHistory %= addREPLItem (REPLOutput out)
invalidateCacheEntry REPLHistoryCache
vScrollToEnd replScroll
gameState . replStatus .= REPLDone (Just val)
gameState . baseRobot . robotContext . at itName .= Just val
Expand Down Expand Up @@ -1129,7 +1130,9 @@ handleREPLEventTyping = \case

if not $ s ^. gameState . replWorking
then case repl ^. replPromptType of
CmdPrompt _ -> runBaseCode topCtx uinput
CmdPrompt _ -> do
runBaseCode topCtx uinput
invalidateCacheEntry REPLHistoryCache
SearchPrompt hist ->
case lastEntry uinput hist of
Nothing -> uiState %= resetREPL "" (CmdPrompt [])
Expand Down
2 changes: 2 additions & 0 deletions src/Swarm/TUI/Model/Name.hs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ data Name
WorldEditorPanelControl WorldEditorFocusable
| -- | The REPL input form.
REPLInput
| -- | The REPL history cache.
REPLHistoryCache
| -- | The render cache for the world view.
WorldCache
| -- | The cached extent for the world view.
Expand Down
4 changes: 2 additions & 2 deletions src/Swarm/TUI/Model/Repl.hs
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ getLatestREPLHistoryItems n h = toList latestN

-- | Get only the items from the REPL history that were entered during
-- the current session.
getSessionREPLHistoryItems :: REPLHistory -> [REPLHistItem]
getSessionREPLHistoryItems h = toList $ Seq.drop (h ^. replStart) (h ^. replSeq)
getSessionREPLHistoryItems :: REPLHistory -> Seq REPLHistItem
getSessionREPLHistoryItems h = Seq.drop (h ^. replStart) (h ^. replSeq)

data TimeDir = Newer | Older deriving (Eq, Ord, Show)

Expand Down
8 changes: 6 additions & 2 deletions src/Swarm/TUI/View.hs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import Control.Lens as Lens hiding (Const, from)
import Control.Monad (guard)
import Data.Array (range)
import Data.Bits (shiftL, shiftR, (.&.))
import Data.Foldable (toList)
import Data.Foldable qualified as F
import Data.Functor (($>))
import Data.IntMap qualified as IM
Expand Down Expand Up @@ -1330,13 +1331,16 @@ renderREPLPrompt focus repl = ps1 <+> replE
drawREPL :: AppState -> Widget Name
drawREPL s =
vBox
[ withLeftPaddedVScrollBars . viewport REPLViewport Vertical . vBox $ history <> [currentPrompt]
[ withLeftPaddedVScrollBars
. viewport REPLViewport Vertical
. vBox
$ [cached REPLHistoryCache (vBox history), currentPrompt]
, vBox mayDebug
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This vBox of empty list is a neat trick, I am surprised it does not produce an empty line. 🤔

@byorgey I noticed I can no longer click in the middle of the REPL to focus on it, only the edges work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed I can no longer click in the middle of the REPL to focus on it, only the edges work.

Good catch. I looked into this for a while but can't figure it out. It seems like after making the REPL into a scrollable viewport, click events are no longer generated for any clicks in the interior of the panel. I'm wondering if it's a brick bug. I might just split this out into another issue and investigate it later.

]
where
-- rendered history lines fitting above REPL prompt
history :: [Widget n]
history = map fmt . getSessionREPLHistoryItems $ repl ^. replHistory
history = map fmt . toList . getSessionREPLHistoryItems $ repl ^. replHistory
currentPrompt :: Widget Name
currentPrompt = case (isActive <$> base, repl ^. replControlMode) of
(_, Handling) -> padRight Max $ txt "[key handler running, M-k to toggle]"
Expand Down