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

[wicket] correctly wrap popup text within step logs #3081

Conversation

sunshowers
Copy link
Contributor

@sunshowers sunshowers commented May 10, 2023

Previously, we were relying on tui's paragraph wrap functionality. This
had several limitations:

  1. There's no way to obtain the actual height of the text as computed.
    This meant that wrapped text was previously cut off.
  2. There's no sensible way to implement scrolling.

To address these issues, we implement wrapping ourselves, via the
textwrap library. The library exposes support for wrapping strings, but
we're working with tui Spans (which have styles associated with them) so
we have to do some work ourselves. Thankfully, it isn't too much work:
textwrap exposes an abstract interface called Fragment, so it's mostly
a matter of copy-pasting some code from textwrap, and annotating each
word with its corresponding tui style.

This PR does not implement scrolling, but now that we're working with
pre-wrapped text in the popup, scrolling should be pretty simple to do
in a followup.

Screenshot of wrapped text:

image

Created using spr 1.3.4
Created using spr 1.3.4
Created using spr 1.3.4
}

fn whitespace_width(&self) -> f64 {
// Since whitespace is always ASCII spaces, this is
Copy link
Contributor

Choose a reason for hiding this comment

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

this is... "equal to the number of whitespace characters", maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in #3126.

@jgallagher jgallagher merged commit 8b0ab46 into main May 11, 2023
@jgallagher jgallagher deleted the sunshowers/spr/wicket-correctly-wrap-popup-text-within-step-logs branch May 11, 2023 15:34
sunshowers added a commit that referenced this pull request May 16, 2023
Deduplicate nested events coming from remote machines in a couple of spots:

1. When we're generating nested events, such that duplicate events don't make their way over the event channel. Maintain event buffers within `StepContext`s to handle that.
2. In `EventBuffer` itself, dedup events based on the leaf index rather than the root index. If two events have the same leaf event index and leaf execution ID, they can always be deduped.

While testing this, I also realized it would be really useful to add a `root_execution_id` to every event report: do so, and depend on it (note that you'll have to rebuild installinator with this PR to pick up support for that).

Finally, address the review comment in #3081 -- do it here to avoid having to create a whole separate PR for this.

Fixes #3102.
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