gyscos / cursive

A Text User Interface library for the Rust programming language
MIT License
4.31k stars 247 forks source link

A few ideas from developing a non-trivial application with cursive #519

Open htejun opened 4 years ago

htejun commented 4 years ago

I developed resctl-demo using cursive. First of all, cursive was a joy to learn and use and the project wouldn't have been possible without cursive. Thank you so much.

Here are some ideas I had while working on it, some of which I'm sure is from me not knowing what I'm doing:

Colors

I couldn't figure out how Rgb colors are mapped to terminal colors. The color values will get applied with high variance depending on the terminal configuration. e.g. gnome-terminal + tmux would yield one set of colors but the colors would change without tmux in the middle. If I keep tweaking colors, it'd change syntax highlighting colors in emacs in a different tmux pane too, so I suppose shared palette is involved but it's all a black magic to me. This posed a bit of challenge because I was getting wildly different brightness of a color depending on the terminal type. I read about true color support in terminals but couldn't figure out how to detect or support it.

Focus and scrolling in LinearLayout::vertical()

resctl-demo has an interactive document pane on the right which is a vertical LinearLayout containing TextViews, Buttons and Checkboxs and SliderViews. PgUp/Down and arrow up/down navigation generally works but the focus position can be outside the current viewable area and the behavior becomes a bit jarring. ie. if I scroll up so that the focus is way below in the doc and then press arrow-down, the view will jump right to where the focus now is. I worked around this by adding dummy buttons between paragraph but it'd be really great if there's an option to keep focus within the current viewable area.

Focus management / key-binding

I'm sure this is doable but I couldn't figure out how to block some elements from getting focus. There are two small scrollable areas left to the main doc area and they would get focus if the user presses left arrow (which can happen quite a bit when the user is interacting with a slider) which can get pretty confusing. It'd be great if I could e.g. restrict pane-jumping to tab and focus navigation in the pane to arrows.

Borderless Panel

When g is pressed in resctl-demo, the app switches to graph view. I tried to get the graph view replace the content portion (lower 3/4 of the interface) while leaving the top status portion unchanged. I couldn't find a way to draw the graphs without extra Panel boundaries. I wonder whether borderless Panels can be useful.

call_on_name() on multiple targets

The doc pane can have, e.g., multiple sliders for the same control value. When the value changes, all have to change together. I worked around by assigning different names to them but it could be useful to be able to look up and call on multiple targets which share the same name.

LogViewer using TextView

TextView supports replacing whole and appending but not trimming. For the log viewer, I'm keeping replacing the whole content whenever the content changes which isn't optimal. It'd be cool to have a way to trim content from TextView.

gyscos commented 4 years ago

Hi, and thanks for the very detailed report! This kind of insight is super valuable to help improve Cursive. :heart:

Colors

I suppose you're using the default ncurses backend? This backend tries to be smart and detects the colors supported by the terminal. It relies on the $TERM variable for that, and on an accurate terminfo database. The problem is that:

You can try other backends like crossterm or termion - these are much simpler and do not try to read the terminal capabilities, they just always output full "true color" and hope the terminal is fine with that.

Focus and scrolling

Yeah it may not be perfect yet. The current logic is:

Focus management / key-binding

That's a good point! Right now arrows and tabs have a relatively similar behaviour, so making them access different items is not trivial. That being said, they do currently have a different "focus source", so maybe we could use that to control what gets focused? Will need to think about it for a bit.

Borderless Panel

Well, a Panel is really only useful for its border, and maybe its title. What are you trying to achieve? Maybe just using the graphview without wrapping it in a Panel? Note that in addition to borders, by default layers have shadows around them. If the style removes shadows, there is still a 1-cell invisible border around layers - that is, unless you use add_fullscreen_layer, which skips the shadow wrapper. I understand it's terrible to discover, and not super convenient to use if you want the view to not be fullscreen.

call_on_name() on multiple targets

Interesting! I could start looking into that.

Trimming TextView

Yeaaah right now it's partly caused by the same limitation on StyledString, but:

Well, thanks again, and hopefully we can make it a bit easier to use! :)

htejun commented 4 years ago

Hi, and thanks for the very detailed report! This kind of insight is super valuable to help improve Cursive.

Glad to hear that I wasn't just bothering you. :)

Colors

I suppose you're using the default ncurses backend? This backend tries to be smart and detects the colors supported by the terminal. It relies on the $TERM variable for that, and on an accurate terminfo database. The problem is that:

Yes, ncurses.

  • $TERM is often incorrectly overriden, either by shell scripts or terminal multiplexer configuration
  • The terminfo database is often incomplete, and may not accurately report the actual color support of a terminal. Especially for terminal multiplexers, where the actual support end up depending on the multiplexer and the parent terminal.

Yes, we have another cursive application which corrupts the screen when scrolling tree view depending on what terminal is being used and TERM. Which combinations work and which don't are still a complete mystery to us. We'll describe the issue fully when it gets opensourced.

You can try other backends like crossterm or termion - these are much simpler and do not try to read the terminal capabilities, they just always output full "true color" and hope the terminal is fine with that.

Yes, I did try other backends but ended up with worse results. For example, the following is with crossterm:

https://drive.google.com/file/d/1Jm7oKqJJrb1BWuowjCi1DhXuolc7zQCV/view?usp=sharing

Focus and scrolling

Yeah it may not be perfect yet. The current logic is:

  • Arrow keys change focus, and scrolling is then adjusted to the focus. This means the focus controls the scrolling, and not the other way around.
  • To still be able to see things out of focus, some actions can let the focus leave the current viewport. This includes scrolling with the mousewheel, and (currently) using Ctrl + the arrow keys.

One weird thing was that if I set the scroll strategy to StickToTop, arrow down would then scroll the page line-by-line through unfocusable elements until it hits the first active target. Then, it'd start jump between focusable targets rather than scrolling line-by-line. For resctl-demo, arrows scrolling content line-by-line when there isn't anything focusable did work well but I think the behavior didn't hold after hitting the first target and got confusing.

  • Something not currently supported is "keeping the focus in view", which means that when we scroll until the focus gets out of view, we try to find another item in view to focus instead. If that's what you're interested in, I can try to look into that.
    • Or maybe just, when changing focus and the current focus is out of view, do not start from the current focus, but from the current viewport?

At least for resctl-demo, it'd work better if up and down arrows always scroll gradually while always keeping focus within the viewable area - if focusable elements are there, focus on them and then move on and keep scrolling up and down so that the user doesn't lose orientation from sudden jumps.

Focus management / key-binding

That's a good point! Right now arrows and tabs have a relatively similar behaviour, so making them access different items is not trivial. That being said, they do currently have a different "focus source", so maybe we could use that to control what gets focused? Will need to think about it for a bit.

Borderless Panel

Well, a Panel is really only useful for its border, and maybe its title. What are you trying to achieve? Maybe just using the graphview without wrapping it in a Panel? Note that in addition to borders, by default layers have shadows around them. If the style removes shadows, there is still a 1-cell invisible border around layers - that is, unless you use add_fullscreen_layer, which skips the shadow wrapper. I understand it's terrible to discover, and not super convenient to use if you want the view to not be fullscreen.

Yes, I wasn't describing the problem correctly. I was trying to get rid of the border because it made the layout look weird with the content narrowed by the extra cell. I wanted to switch a part of the screen with different content but couldn't do so without adding the extra boundary and eventually just used add_fullscreen_layer and replaced the whole screen. So what I was trying to say was that it'd be great to have a borderless layer which is not fullscreen.

call_on_name() on multiple targets

Interesting! I could start looking into that.

Trimming TextView

Yeaaah right now it's partly caused by the same limitation on StyledString, but:

  • We could probably enable trimming a StyledString
  • We might work on a dedicated LogView or something which doesn't have the same restrictions.

I see. Either sounds great to me.

Well, thanks again, and hopefully we can make it a bit easier to use! #:)

Thank you so much again for the great library. Many of us are smitten by your work.

gyscos commented 4 years ago

So on the latest main branch, there is now:

This should let you remove content from a TextView without having to re-set the whole thing.

htejun commented 4 years ago

I'll try using them and see how it goes. I'll try to get back to you before the end of next week. Thank you so much.

htejun commented 4 years ago

I tried using call_on_all_named() but hit a couple obstacles:

Thanks.

gyscos commented 4 years ago

Ah yes these are changes since the last published version.

The cursive-tabs create doesn't build. Getting help from the author.

Working with dependencies on a git version is always painful :(

The Cursive object no longer relies on the backend being alived the entire time, so some actions likes screen_size() may not always be available.

htejun commented 4 years ago

resctl-demo changes layout based on screen width - when it's wide enough, it uses two columns of panels; otherwise, single. Right now, it also uses screen size to determine how big each panel and graph should be but maybe that can be done from the size of each widget. Anyways, yeah, last_size() should work.

refresh() was mostly useful to force full redraw when I was getting screen corruptions from terminal setting / compatibility issues, something outputting warnings / notices to tty, and so on. IIRC, regular navigation screen movements or other keybindings didn't refresh the whole screen. I could be misremembering tho. If a noop global binding does the job, that's great.

gyscos commented 4 years ago

Latest commit should bring back Cursive::screen_size() based on the last layout phase. Note that in general, changing the layout based on available size is exactly what View::layout() is supposed to do: you'd have a root view which checks the size available during layout, and possibly re-organize its children. Though I understand that in some case it's easier to just check the size of the screen without having to build an isolated view.

Re: refresh, I'm curious if you do see a change. If you still see the screen failing to refresh after any event, then it's most likely a bug that we'll need to fix.

htejun commented 4 years ago

With refresh commented out, it builds but doesn't seem to get event::Event::WindowResize anymore which is what triggers the layout update. If I trigger the code path manually, it works fine, including the new call_on_all_named(), which is fantastic. I ended up going for custom layout because I had difficulties with distributing real estate between graphing and log areas, most likely because I didn't know what I was doing. I'm a bit occupied right now but will look into converting back to using normal layout mechanism.

gyscos commented 4 years ago

Ah yes, the WindowResize now has a "pre-event" callback set by default to clear the screen, which effectively prevents the event from being seen (or intercepted) by the view tree.

It's still recommended to use the View::layout phase to detect changes in the size given (theorically, the given size may change without the window resizing), but you can still listen to the WindowResize event; you just need to register a "pre-event" callback using Cursive::set_on_pre_event. Alternatively, if you want the event to be sent down the tree view and/or available for regular callbacks, you'll need to clear the existing default callback first with Cursive::clear_global_callback (or use Cursive::set_global_callback which clears any pre-existing callback). Just be sure to include Cursive::clear in the new callback.

htejun commented 3 years ago

Sorry about the long delay. Here are a couple updates.

The new code based on the latest cursive is in https://github.com/htejun/resctl-demo/tree/on-cursive-tip. I'll now try to use the new StyledString trimming for log view.

Thanks and have a great thanksgiving!

gyscos commented 3 years ago

Ah blast, seems like it's ncurses being smart and having its own internal cache, omitting output that it knows "is already there". Only it's not aware of other processes writing to the tty, and only a manual clear will make it deign re-sending that output. Termion or crossterm, for example, would not have this issue. Though caching is in many cases the right thing to do, and cursive_buffered_backend would bring the same issue as ncurses here.

I suppose this is indeed one of the few cases where a manual clear() is the correct thing to do.

Re: screen_size, I see the problem now: you are updating the layout in the callback for WindowResize, so before a new layout phase was actually started. Yeah View::layout is really the place for this kind of layout updates, but I suppose the WindowResize event is an alluring footgun. Maybe we should unconditionally fetch the new terminal size on each resize event, and include the new size inside the WindowResize?... It would be a bit more work if multiple WindowResize happen between two frames (we'd fetch the size N times instead of once), but it may reduce the confusion from trying to react to this event.

htejun commented 3 years ago

Okay, just pushed out log view implementation w/ incremental addition & compaction to https://github.com/htejun/resctl-demo/commits/on-cursive-tip. A couple notes:

Overall, it worked great. Thank you so much for implementing the features so quickly and helping me along the way.