-
Notifications
You must be signed in to change notification settings - Fork 52
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
add word wrap capability to luma.core.virtual.terminal #78
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need altering too: https://github.com/rm-hull/luma.core/blob/master/setup.py#L40 ?
luma/core/virtual.py
Outdated
self.puts(text) | ||
self.newline() | ||
if self.word_wrap: | ||
text = wrap(text, width=self.width) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does wrap work with ANSI color codes? Would be worth adding a test to check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's def not correct, 'hello world' line is missing. hmm any ideas @rm-hull?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello world is missing because it has scrolled off the top. Clearly the word wrap is counting the ANSI colour code characters rather than interpreting them as zero width.
Wonder whether we need a home grown text wrapper that does take ANSI color codes into account?
Maybe just leave as-is for now.
Interestingly, it has exposed another bug (which was there before your commits) when scrolling when the background colour is not black - see the yellow bar at the bottom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to look into the issue a bit more before merging, thanks for the feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interestingly, it has exposed another bug (which was there before your commits) when scrolling when the background colour is not black - see the yellow bar at the bottom.
I've opened #79 for that issue.
doc/api-documentation.rst
Outdated
@@ -37,6 +37,13 @@ API Documentation | |||
:undoc-members: | |||
:show-inheritance: | |||
|
|||
:mod:`luma.core.interface.serial` | |||
""""""""""""""""""""""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think the underline needs to be the same length as the title else doc generation will complain. we should probably add docgen to the toxic was task if possible
luma.core.interface
package (Add 4- & 8-bit parallel interfaces #41)luma.core.serial
toluma.core.interface.serial
luma.core.serial
module