Fix CLI screen corruption #1704
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The CLI would often have temporary screen corruption. This reworks several things to correct that issue, although it looks like the ultimate cause was drawing to the screen while in the middle of waiting on a VT100 ESC keycode sequence. * Rearchitected all screen drawing to run in a single thread. Now call set_screen_dirty() instead of draw_screen() * Added a lock on accessing the various Log buffers, preventing changes to the buffer in the middle of a redraw * Modified key reading logic when ESC character is received. Now uses a 1 second timeout if no subsequent keycodes are sent
|
Hello @penrods! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on July 28, 2018 at 06:25 Hours UTC |
penrods
added
the
CLA: Yes
Curses throws exceptions in some cases during get_wch(), such as when you Ctrl+Z suspend the CLI the resume the process. Also moved Ctrl+C processing into this exception handler.
KathyReid
approved these changes
Jul 30, 2018
There was a problem hiding this comment.
Test sequence
Pull the PR:
kathyreid@kathyreid-N76VZ:~/Dropbox/PHPworkspace/mycroft-core$ git status
On branch feature/fix-cli-corruption
Your branch is up-to-date with 'origin/feature/fix-cli-corruption'.
Untracked files:
(use "git add <file>..." to include in what will be committed)
settings.json
nothing added to commit but untracked files present (use "git add" to track)
kathyreid@kathyreid-N76VZ:~/Dropbox/PHPworkspace/mycroft-core$ git log
commit 5641f578c0d9efe76ccfab5406fbbfc0f47559ba
Author: Steve Penrod <steve.penrod@mycroft.ai>
Date: Sat Jul 28 01:25:46 2018 -0500
Fix Codacy issues
launch ./start-mycroft debug
mycroft-cli-client launched successfully
- Press Pg Up => correctly scrolls up the backscroll
- Press Pg Down => correctly scrolls down the backscroll
- Repeated pressing of Pg Up or Pg Down => scrolls multiple times correctly
- Hold Pg Up or Pg Down => continuous scroll
- Mashed keyboard like a toddler => printable characters displayed in Input prompt, then
KeyboardInterruptcaptured, dropped out of CLI
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The CLI would often have temporary screen corruption. This reworks
several things to correct that issue, although it looks like the
ultimate cause was drawing to the screen while in the middle of
waiting on a VT100 ESC keycode sequence.
call set_screen_dirty() instead of draw_screen()
changes to the buffer in the middle of a redraw
uses a 1 second timeout if no subsequent keycodes are sent
How to test
Start the CLI and hold the PgUp / PgDown key with a full log. It would previously corrupt the screen.
Contributor license agreement signed?
CLA [ X ] (Whether you have signed a CLA - Contributor Licensing Agreement