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

don't truncate multi-line printf in std renderer #784

Conversation

Adjective-Object
Copy link
Contributor

When logging something into the scrollback history of the standard renderer, the current approach temporarially increases the size of the UI by the number of logged messages. This has 2 issues:

  • log lines are truncated according to the current width of the terminal UI. This truncation behaviour is desireable when writing into the TUI's controlled space, but is not desireable when Printf()ing messages into the scrollback, since the contents of the logged line will be truncated with no way to recover the history.
  • User code pre-wrapping messages they want to print to the scrollback don't behave the way that you would want/expect log lines in a terminal to behave:
    • Resizing the terminal emulator does not reflow the lines to the new terminal's width
    • Manual wrapping breaks most terminal emulator's detection of clickable URLs and file paths

This change replaces that approach with a full clear of the previos UI + printing unwrapped messages into the terminal before the next render

this has a minor performance impact because it necessitates a full clear of the terminal UI on each Printf(). This breaks duplicate line detection between renders. However, the current approach will rarely hit duplicate line detection, since adding log messages to the front of the printout buffer offsets the new buffer. Therefore, the current line detection logic will only trigger when the buffer contains duplicate lines offset by the number of messages being logged in this render pass.

@meowgorithm
Copy link
Member

Thanks for the detailed writeup and implementation (and patience here). This all makes a lot of sense; we'll review shortly.

Max Huang-Hobbs added 2 commits February 8, 2024 20:43
When logging something into the scrollback history of the standard renderer, the current approach temporarially increases the size of the UI by the number of logged messages. This has 2 issues:

- log lines are truncated according to the current width of the terminal UI. This truncation behaviour is desireable when writing into the TUI's controlled space, but is not desireable when Printf()ing messages into the scrollback, since the contents of the logged line will be truncated with no way to recover the history.
- User code pre-wrapping messages they want to print to the scrollback don't behave the way that you would want/expect log lines in a terminal to behave:
  - Resizing the terminal emulator does not reflow the lines to the new terminal's width
  - Manual wrapping breaks most terminal emulator's detection of clickable URLs and file paths

This change replaces that approach with a full clear of the previos UI + printing unwrapped messages into the terminal before the next render

this has a minor performance impact because it necessitates a full clear of the terminal UI on each Printf(). This breaks duplicate line detection between renders. However, the current approach will rarely hit duplicate line detection, since adding log messages to the front of the printout buffer offsets the new buffer. Therefore, the current line detection logic will only trigger when the buffer contains duplicate lines offset by the number of messages being logged in this render pass.
@Adjective-Object
Copy link
Contributor Author

Duplicate of #490

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.

2 participants