Skip to content

RAW UTF-8 terminal: SI/SO/IBMPC_ON/IBMPC_OFF skip #787

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 7, 2024
Merged

RAW UTF-8 terminal: SI/SO/IBMPC_ON/IBMPC_OFF skip #787

merged 1 commit into from
Feb 7, 2024

Conversation

penguinolog
Copy link
Collaborator

  • In the case of UTF-8 terminal, charset manipulation sequences are not needed.
  • In the case of no style changes, not needed to set style again each line

Validated:

  • Linux console
  • FreeBSD console
  • XTerm
  • RXVT
  • VTE-based terminals
  • Alacritty
  • JetBrains IDE console
  • WSL 2
  • Windows 11 cmd & powershell
Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment
  • I've included docstrings and/or documentation and/or examples for my code (if this is a new feature)

@penguinolog penguinolog added the Unicode Issues related to Unicode <-> bytes conversion label Feb 2, 2024
@github-actions github-actions bot added the docs Issues related to documentation label Feb 2, 2024
@coveralls
Copy link

coveralls commented Feb 2, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 7796628429

  • -9 of 9 (0.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.008%) to 70.93%
Changes Missing Coverage Covered Lines Changed/Added Lines %
urwid/display/_raw_display_base.py 0 9 0.0%
Files with Coverage Reduction New Missed Lines %
urwid/display/_raw_display_base.py 1 22.7%
Totals Coverage Status
Change from base Build 7796558930: -0.008%
Covered Lines: 8769
Relevant Lines: 12453
💛 - Coveralls

@penguinolog
Copy link
Collaborator Author

Made extra synthetic tests for partial render: no signs of attributes mixing during render.

@penguinolog
Copy link
Collaborator Author

@danschwarz please look at this.
This changes worked well for me on the listed terminals, but something may be missed. I feel that you are better in the low level terminal operations.

penguinolog
penguinolog commented Feb 2, 2024
View reviewed changes
examples/palette_test.py Outdated
# mode radio buttons
rb = urwid.RadioButton(mode_radio_buttons, text, state)
urwid.connect_signal(rb, "change", on_mode_change, colors)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecated logic for a while

urwid/canvas.py Outdated
@@ -560,7 +562,7 @@ class BlankCanvas(Canvas):
"""

def __init__(self) -> None:
super().__init__(None)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect call (all arguments for the Canvas class are deprecated)

urwid/display/_raw_display_base.py Outdated
@@ -531,6 +531,40 @@ def _setup_G1(self) -> None:
def draw_screen(self, size: tuple[int, int], r: Canvas) -> None:
"""Paint screen with rendered canvas."""

def set_cursor_home() -> str:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the top of method

urwid/display/_raw_display_base.py
o = [escape.HIDE_CURSOR, self._attrspec_to_escape(AttrSpec("", ""))]
last_attributes = None # Default = empty

output = [escape.HIDE_CURSOR, attr_to_escape(last_attributes)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attr_to_escape with None should set default attributes. It's enough for normal and incremental draw.

urwid/display/_raw_display_base.py

if not (IS_WINDOWS or IS_WSL) and (first or lastcs != cs):
if encoding != "utf-8" and (first or last_charset_flag != cs):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of WSL and Windows, charger is always utf-8. Windows is mentioned in the python stdin/stdout/stderr documentation explicit, WSL is well known.
On the listed in the PR comment terminals this rules worked perfectly with non-ascii symbols to display.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My experience is mostly in VT100/xterm type terminals rather than the Windows command.com environment, but I'll take a look.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows part is good, "normal" modern terminals also good. But I'm not 100% sure about old terminals.
Reduced amount of escape sequences seriously reduces future work pain, but I do not want to break something.
In case of this PR fully OK, we can detach final encoding for render from Canvas (final composition & screen-independent render).

@penguinolog penguinolog force-pushed the raw_display_optimization branch from 4438e07 to b152412 Compare February 5, 2024 07:51
RAW UTF-8 terminal: SI/SO/IBMPC_ON/IBMPC_OFF skip
5a1d1df 
* In the case of UTF-8 terminal,
  charset manipulation sequences are not needed.
* In the case of no style changes,
  not needed to set style again each line
* We know the first attributes since we set it on init

Validated:
* Linux console
* FreeBSD console
* XTerm
* RXVT
* VTE-based terminals
* Alacritty
* JetBrains IDE console
* WSL 2
* Windows 11 cmd & powershell
@penguinolog penguinolog force-pushed the raw_display_optimization branch from b152412 to 5a1d1df Compare February 6, 2024 08:18
@penguinolog
Copy link
Collaborator Author

Check on the all not pre-historic terminals passed, prehistoric terminals do not support UTF-8 and receive all escape sequences as before.

@penguinolog penguinolog merged commit 8d81c27 into urwid:master Feb 7, 2024
@penguinolog penguinolog deleted the raw_display_optimization branch February 7, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danschwarz danschwarz danschwarz left review comments

Assignees
No one assigned
Labels
docs Issues related to documentation Unicode Issues related to Unicode <-> bytes conversion
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@penguinolog @coveralls @danschwarz