feat(tui): configurable scrollback length #10247
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| }; | ||
|
|
||
| const SCROLLBACK_LEN: usize = 1024; | ||
| const SCROLLBACK_LEN: usize = 2048; |
There was a problem hiding this comment.
Making sure to call extra attention here: Users mentioned they felt 1024 was too small, so I doubled it here. Not sure about if there's a reason I shouldn't do this or if a different value should be used. Just did this for the sake of discussion.
There was a problem hiding this comment.
I think this is okay since #9123 has been merged.
vt100 used to need to iterate through every scrollback row in order to render a visible row so 1024->2048 would be a noticeable perf hit. I will do a quick profile to check if there's a noticeable impact from this change.
There was a problem hiding this comment.
Did a test and this is a no noticeable perf change
|
Thanks @anthonyshew this looks great! 🙏🙇 We could also maybe fallback to the |
chris-olszewski
left a comment
There was a problem hiding this comment.
If we're gonna add a config option, then we should do it right. (Gotta keep the dream of code generated docs alive)
|
@lbennett-stacki I don't think we want to use
https://www.gnu.org/software/bash/manual/html_node/Bash-History-Facilities.html |
It's not a dream, it's real. /s
I wasn't going to do so in this PR. I was just going to leave it at the env var. Are you saying we should do one? |
Sorry, I meant "config option" in the most general sense of a way to configure how |
|
Using #10261 to break things up into PRs that only do one thing. |
### Description Breaking up #10247 into PRs that only do one thing. This handles the bump for default scrollback length. Performance was assessed here: #10247 (comment) ### Testing Instructions 👀
|
@chris-olszewski, I refactored. Can you take another look? |
Description
Allow users to set a different value for scrollback length. I'm also taking a liberty to double the scrollback length's default value here (cc @chris-olszewski to tell me if this isn't a good idea)
Original asks:
Note
If you're wondering why we don't dynamically set this to whatever the terminal's scrollback is, terminals don't expose this value. The closest thing to a spec for terminals is the xterm spec, so we don't know what we can depend on.
Questions
Should this also be:
turbo.jsonconfig?turbo.jsontends to be that it should be something that makes sense for anyone in the repo, and setting some non-default scrollback length doesn't sound desirable.Testing Instructions
Try it!