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

Implement non-fullscreen streaming rendering #13

Closed
wants to merge 2 commits into from

Conversation

quark-zju
Copy link
Contributor

@quark-zju quark-zju commented Jan 5, 2020

This matches less -F -X behavior before entering the full screen - lines are printed as the pager recieves them.

Demo: https://asciinema.org/a/KZbc9K1elnz9alqxbCq6k1XLK

I had a hard time naming the feature. Better names are welcome.

@markbt
Copy link
Owner

markbt commented Jan 6, 2020

This removes the delay fullscreen mode, which is my preferred mode of operation (and, in fact, my primary motivator for writing sp). I want to use alternate screen mode if and only if the pager is necessary. I'm happy to pay the ~2s delay of getting no output before the pager kicks in (in practice I find most things are either <1s or much longer).

I'm happy to add new modes, but please don't remove delay :)

@quark-zju
Copy link
Contributor Author

By using the alternate screen, do you mainly want the behavior of cleaning up the pager content on exit? If that's the case, how about:

  • Still print content immediately before entering full screen.
  • When entering full screen, erasing lines printed so it appears nothing was printed in the non-alternate-screen.
  • Always use the alternate screen for the full screen mode.

@markbt
Copy link
Owner

markbt commented Jan 6, 2020

That's part of it. I do like how it leaves the terminal output clean after the pager exits, so if we can emulate that, too, that'll be much better than less's current behaviour.

The other part is for commands that produce output slowly. By jumping into the fullscreen pager after 2s, it's possible to see that something is happening, and also to do things like search or scroll through the limited amount of output that's already there. It's quite nice to see the animated throbber in the bottom left, plus have the new content pop into view as it comes in.

The latter one is probably acceptable if we make any keypress jump into the fullscreen pager, too. However, I have been using the delay mode for a year now, and I am quite fond of it's behaviour.

@quark-zju
Copy link
Contributor Author

I got a version with the delayed mode integerated. I will update this after cleaning it up.

Emulates `less -F -X`. Stream the output immediately to the terminal before
entering the full screen mode. Error and progress are rendered after output
so it looks visually similar to the full screen UI.

The streaming rendering mode (`InterfaceMode::Hybrid`) can be used standalone
without entering full screen (`InterfaceMode::Cat`) to emulate buck-build-style
output.

The 2-second delay "all or nothing" rendering is preserved as
`InterfaceMode::Delayed`.
@quark-zju quark-zju force-pushed the less-fx branch 3 times, most recently from 9c74382 to 9481cfd Compare January 8, 2020 05:13
@quark-zju
Copy link
Contributor Author

I added InterfaceMode::Delayed for the delayed behavior. I added some docstrings on InterfaceMode options. I figured the streaming mode is more similar to cat than tail -f. So I renamed the module to cat.rs.

Add flags `--delayed (-D)`, `--no-alternate (-X)` for the new InterfaceMode
config. The old flag `--force` was removed. It can be expressed by `-D 0`.

`sp -X` matches `less -X -F`.
`sp -D 9999` roughly matches `less -F`.
`sp -D 0` matches `less` without `-X` or `-F`.

`sp` without arguments works like `sp -D 2`, preserving the old behavior.
@markbt
Copy link
Owner

markbt commented Jan 11, 2020

Thanks! This looks good.

Naming things is hard. What do you think about the name "direct" instead of "cat" to represent rendering directly to the terminal (rather than entering fullscreen mode)?

If you're happy with that, I'll do a search+replace and then merge it.

@quark-zju
Copy link
Contributor Author

Yeah, I'm not too happy with cat. direct looks good to me.

In some distince future I think it's more consistent for the "direct" interface to also support features like searching and showing line numbers. I didn't find an easy way to reuse Screen to do that, though.

@markbt
Copy link
Owner

markbt commented Jan 11, 2020

Cool. Watching the video again, one idea might be for the non-alt-screen version to clear off the overlay lines at the bottom of the screen when you quit, so only the content is left after the program exits. That can happen later though, no need to do that now. I'll see if I can find some time to do the search+replace tomorrow, and then merge these two requests.

Thanks again!

@quark-zju
Copy link
Contributor Author

I actually had the "clear overlay lines" implemented in a draft branch targeting at "theming" support (ex. options to behave like less - search and ruler uses a same line), where it switches to a "blank" theme to remove the overlays at exit. I found it difficult to get all line offset and refresh calculation done right cleanly, though. A React-like pattern might help, while preserving ScrollRegion fast paths can be challenging to implement cleanly.

@markbt markbt closed this in d204826 Jan 12, 2020
markbt added a commit that referenced this pull request Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants