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

Replace tcell with roll-our-own #42

Closed
wants to merge 5 commits into from
Closed

Replace tcell with roll-our-own #42

wants to merge 5 commits into from

Conversation

walles
Copy link
Owner

@walles walles commented Apr 14, 2021

Before this change, the main loop was roughly:

  • Await one single user input event
  • Handle single user input event
  • Redraw screen

With this change in place, the main loop is:

  • Await one single user input event (like before)
  • Handle single user input event (like before)
  • If there are no more events, redraw screen (new!)

The difference is huge if you do a fling scroll on a trackpad. Before, scrolling was slow at constant speed, and the scrolling kept on at that slow constant speed way longer than what it should have. Now, scrolling feels just like it should.

@walles
Copy link
Owner Author

walles commented Apr 14, 2021

@89z could you test this on Windows?

If you could just build moar from this branch and try using it to view some file that would be swell!

You provided Windows screenshots in #33 so I assume you have a Windows machine with a Go environment set up.

@walles
Copy link
Owner Author

walles commented Apr 15, 2021

@89z thanks!

I excluded SIGWINCH handling on Windows, if you could make another try that would be awesome!

@walles
Copy link
Owner Author

walles commented Apr 15, 2021

Build + unit testing now pass on Windows, I added a Travis config so any breakage should become obvious.

Could still be visually all-broken though, needs testing by a human on an actual Windows machine.

Just starting moar on some file and verifying...

  • Scroll down
  • Quit

... should cover most Windows specific cases.

@walles
Copy link
Owner Author

walles commented Apr 15, 2021

I think it's because of this, no explicit access to /dev/stdout is ever done:

moar/twin/screen.go

Lines 105 to 114 in 7cd1571

// os.Stdout is a stream that goes to our terminal window.
//
// So if we read from there, we'll get input from the terminal window.
//
// Reading from os.Stdin will fail if we're getting data piped into
// ourselves from some other command.
//
// Tested on macOS and Linux, works like a charm!
screen.ttyIn = os.Stdout // <- YES, WE SHOULD ASSIGN STDOUT TO TTYIN
screen.ttyOut = os.Stdout

I'll see if I can figure out some way of testing this on Windows somehow.

@walles
Copy link
Owner Author

walles commented Apr 17, 2021

Found a Windows machine, will fix things up there.

@walles walles closed this Apr 17, 2021
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.

Fix shutdown hang Avoid screen refreshes
1 participant