Skip to content

Terminal history cleared if output is greater than height of terminal window #382

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

Closed
LeeCheneler opened this issue Sep 22, 2020 · 9 comments

Comments

@LeeCheneler
Copy link

If the terminal height is 10 lines for instance, and my ink app renders out 11 lines then the whole terminal history is wiped 😳

This makes ink unusable for any type of terminal app except ones that want to completely take over the terminal such as jest watch.

@LeeCheneler
Copy link
Author

Am happy to contribute an update for this but having a look through issues and a PR it looks like the change may to wipe history may have been made on purpose. I can see the check that causes it to wipe the terminal history.

Is there a way we can change that behaviour to be opt in? Although I guess now it would be a breaking change 😞 so an alternative would be to turn that behaviour off completely via a render option, hook or component?

Whatever it takes to get this change over the line as currently our only option is to roll back to a previous version of ink and rework the app to suit it whilst we get our initial release out which has had an awful lot of work go into it and then rewrite almost the entire app following initial release using another technology 😅

@vadimdemedes
Copy link
Owner

Yeah, you are right, this behavior is built on purpose. A lot of people got confused why UIs that are taller than terminal's viewport don't render correctly (the part outside of the viewport), that's why I built it to clear the terminal history on every render to ensure all UI is rendered as expected.

@vadimdemedes
Copy link
Owner

So if we remove that, terminal won't render correctly the part that's above the visible area. I don't like that I have to erase the terminal history, but unfortunately I haven't found any other solution, this seems to be how all terminals behave.

@vadimdemedes
Copy link
Owner

@sindresorhus Curious what your thoughts are about this problem!

@LeeCheneler
Copy link
Author

Could a mechanism be added to prevent this from happening? Like a setting passed into the render method?

Right now we only have 3 paths forward if this isn't fixed:

  • Fork and fix (this solution totally sucks)
  • Build our own react render for CLI... Not end of world but will set development back a while
  • rewrite 90% of app and don't use react which would be a disaster as it's helped us build a super rich terminal experience and we won't be able to achieve it very easily with any other tech

@vadimdemedes
Copy link
Owner

But your UI wouldn't render properly with this disabled. The part that's invisible (which you have to scroll up to see) is not going to re-render. You can reproduce this yourself by using log-update and trying to render a tall enough string that's taller than your terminal's height.

Build our own react render for CLI

How would you solve this problem of re-rendering "tall" UIs?

@loilo
Copy link
Contributor

loilo commented Oct 1, 2020

This would probably be solvable by devs explicitly approaching a full screen app (using an alternate screen buffer as described here). That would not allow users to scroll through their history while an Ink app is running, but it would bring back the history after exiting the app.

The main problem I see with this is that Ink is currently not very well suited for this kind of full screen applications: Re-rendering of tall UIs flickers a lot (at least in iTerm2, which I'd argue is widespread enough to make this a problem). This could potentially be fixed on Ink's side for fullscreen apps specifically — by only ever rendering the current viewport. However, that would require the framework to also intercept and re-implement things like scrolling, therefore making the fullscreen app functionality something that would probably have to be integrated into Ink's core (which altogether obviously is quite a lot of work).

@LeeCheneler
Copy link
Author

I've spiked it out now and see what you mean @vadimdemedes. Not really any way around it. I think it would be a good idea to properly explain that it clears the whole terminals contents in renders taller than the terminal in the readme. This is going to set us back a couple of months worth of development as I think we're going to have to completely rebuild the CLI tool 😔

It's such a shame, because only like 2 of our commands are full screen interactive commands, the rest are a single render pass so have no reason to wipe the terminals contents at all. The full screen commands we're perfectly fine with wiping the history as they're interactive command, but it's unacceptable that we wipe the history when we run our other commands 😔

Might still use ink for the interactive sections but I doubt it as we want a consistent rendering technology for the whole app.

Ink still rocks though 🤘 favourite discovery so far this year! Even wrote a command router for it the other month 🙂 any chance of getting it in the tools section in the readme? 😉
https://github.com/LeeCheneler/ink-command-router

@vadimdemedes
Copy link
Owner

This would probably be solvable by devs explicitly approaching a full screen app (using an alternate screen buffer as described here). That would not allow users to scroll through their history while an Ink app is running, but it would bring back the history after exiting the app.

That's an interesting solution, I think it's definitely worth exploring as I'm also not a big fan of erasing people's terminal history. I wonder why Jest didn't adopt it yet, as they're in a similar situation. Does anyone want to try it out and see how it works? Doesn't have to be in Ink's repository, could be a standalone script that uses log-update.

The main problem I see with this is that Ink is currently not very well suited for this kind of full screen applications: Re-rendering of tall UIs flickers a lot (at least in iTerm2, which I'd argue is widespread enough to make this a problem). This could potentially be fixed on Ink's side for fullscreen apps specifically — by only ever rendering the current viewport. However, that would require the framework to also intercept and re-implement things like scrolling, therefore making the fullscreen app functionality something that would probably have to be integrated into Ink's core (which altogether obviously is quite a lot of work).

I agree, Ink is not well-suited for fullscreen UIs and I never intended it too, there's blessed for that.

Ink still rocks though 🤘 favourite discovery so far this year! Even wrote a command router for it the other month 🙂 any chance of getting it in the tools section in the readme? 😉
https://github.com/LeeCheneler/ink-command-router

@LeeCheneler Thanks! ink-command-router seems interesting, feel free to open a PR to add it there.

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

No branches or pull requests

3 participants