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

Ability to scroll the REPL panel #60

Closed
byorgey opened this issue Sep 25, 2021 · 9 comments · Fixed by #1481
Closed

Ability to scroll the REPL panel #60

byorgey opened this issue Sep 25, 2021 · 9 comments · Fixed by #1481
Labels
C-Low Hanging Fruit Ideal issue for new contributors. G-REPL An issue having to do with the REPL. S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. T-UI Involves the user interface. Z-Feature A new feature to be added to the game. Z-User Experience This issue seeks to make the game more enjoyable to play.

Comments

@byorgey
Copy link
Member

byorgey commented Sep 25, 2021

The user can scroll through their history using up/down, but perhaps it would be nice to allow scrolling the entire REPL itself, so they can actually look at what they did in context? I am not sure of the best way to do this (maybe with the mouse?).

@byorgey byorgey added Z-User Experience This issue seeks to make the game more enjoyable to play. Z-Feature A new feature to be added to the game. C-Moderate Effort Should take a moderate amount of time to address. S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. T-UI Involves the user interface. labels Sep 25, 2021
@mateoatr
Copy link

What about Ctr+Shift+? I kind of expect it to work on terminals I use in Unix systems.

@byorgey
Copy link
Member Author

byorgey commented Sep 25, 2021

Ah, good point. I often use Shift+PgUp/PgDown to scroll by pages. I guess Ctrl+Shift+Up/Down is sometimes used to scroll one line at a time? I have never used that and I didn't know about it until just now.

@xsebek
Copy link
Member

xsebek commented Oct 29, 2021

After the refactoring of REPL history (in #253) is merged, this could be achieved by adding an offset to the latest line:

┌────────────────────────────────────────────────────┐
│ > "the 7th line entered"                           │
│ "the 7th line entered" : string                    │
│ > log "the 8th line entered"     // offset is 8    │
│ > log "current input, history is 42 lines long"    │
└────────────────────────────────────────────────────┘

Unless I am missing something this only requires adding Ctr+ Shift+ ?, the offset and generalizing getLatestREPLHistoryItems to respect the offset in drawREPL.

What do you think @byorgey, should I change it to ? 🙂

@byorgey
Copy link
Member Author

byorgey commented Oct 29, 2021

A better way would probably be to render all of the REPL history into a scrollable Viewport and then let brick handle the scrolling. For example, see the way we handle the bottom-left info panel. This way the scroll amount doesn't depend on whether there are wrapped lines, multi-line inputs, etc.

It could get expensive to render the entire REPL history in memory when it gets large. We could have a (configurable?) limit to the amount you can scroll, just like most terminal apps.

@xsebek
Copy link
Member

xsebek commented Oct 29, 2021

I do not see how it would be more expensive then what we do now, currently we have the whole history loaded in the memory so we could share it with brick. 😉

I am not sure if the Viewport is worth it - if we just keep a list of Widgets, then we can easily reuse those lines, for example to show history search results (#85) or keywords to select from with Tab completion (#207). While it would be nice to just let brick handle it, the power to do anything with the REPL is tempting too (for me at least, because I would not have to learn brick 😅 ).

As another though, if we add this history scrolling, then it would be nice to have a separating line above input with the " . . . " like in logger, so it would be obvious that the history is being scrolled.

@byorgey
Copy link
Member Author

byorgey commented Oct 29, 2021

I do not see how it would be more expensive then what we do now, currently we have the whole history loaded in the memory so we could share it with brick.

Well, having brick render the entire history (and then only show a part of it) could be more expensive.

I am not sure if the Viewport is worth it - if we just keep a list of Widgets, then we can easily reuse those lines, for example to show history search results (#85) or keywords to select from with Tab completion (#207). While it would be nice to just let brick handle it, the power to do anything with the REPL is tempting too (for me at least, because I would not have to learn brick ).

You might be right, I don't know. We'll have to play with it.

As another though, if we add this history scrolling, then it would be nice to have a separating line above input with the " . . . " like in logger, so it would be obvious that the history is being scrolled.

Agreed. That is not hard to add.

@byorgey byorgey added the G-REPL An issue having to do with the REPL. label Apr 7, 2022
@byorgey
Copy link
Member Author

byorgey commented Jun 16, 2022

I also note that brick Viewports now support a scrollbar, which would fulfill the same purpose as the . . . line (probably better). I'd like to look into adding scrollbars in general.

@byorgey
Copy link
Member Author

byorgey commented Aug 17, 2022

I think making a scrollable brick Viewport and adding a scrollbar (which we now have) is the way to go, with e.g. Shift-PgUp/Down to scroll. This video linked in another issue may be relevant as well: #629 (comment)

@xsebek
Copy link
Member

xsebek commented Oct 16, 2022

@byorgey OK, I am convinced, let Viewports rule all! 👍

REPL is not as bad as messages, there will be fewer entries and they will not get appended while you scroll. But if we figure out how to do messages well, then REPL could also benefit from a slight speedup. 🙂

@byorgey byorgey changed the title Ability to scroll the REPL panel? Ability to scroll the REPL panel Jan 6, 2023
@byorgey byorgey added C-Low Hanging Fruit Ideal issue for new contributors. and removed C-Moderate Effort Should take a moderate amount of time to address. labels May 12, 2023
@mergify mergify bot closed this as completed in #1481 Sep 3, 2023
mergify bot pushed a commit that referenced this issue Sep 3, 2023
- Add scrollbars on both the inventory and info panels
- Get rid of `. . .` at top and bottom of info panel, since we now have scrollbar as a visual indicator when there is more content
- Allow scrolling the REPL history (closes #60)
    - PgUp/PgDown can be used to scroll (Shift+PgUp/Dn were not recognized on my system)
    - Hitting any other key causes the view to jump back to the very bottom
    - A computation finishing + printing an output also causes the view to jump to the bottom
    - The REPL history is cached so that it only gets re-rendered whenever a new history entry (i.e. input or output) is added; this is needed since the history could get quite large.
- Also, fix the height of the key hint menus to 2 lines, even when the panel-specific menu (second line) is blank, so the world panel does not keep resizing as we move the focus between panels.

Thanks to @jtdaugherty for releasing `brick-1.10` with a new ability to specify blank space to the side of scrollbars; see jtdaugherty/brick#484 .

Also towards #1461 .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Low Hanging Fruit Ideal issue for new contributors. G-REPL An issue having to do with the REPL. S-Nice to have The bug fix or feature would be nice but doesn't currently have much negative impact. T-UI Involves the user interface. Z-Feature A new feature to be added to the game. Z-User Experience This issue seeks to make the game more enjoyable to play.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants