andygrove opened a new pull request, #2014:
URL: https://github.com/apache/datafusion-ballista/pull/2014
# Which issue does this PR close?
Closes #2013.
# Rationale for this change
The TUI banner renders as garbled tofu boxes for most users. `render_banner`
picked its `tui-big-text` `PixelSize` by width, and all three reachable
branches draw from Unicode's *Symbols for Legacy Computing* block, which most
monospace fonts do not carry:
```rust
let banner_size = match area.width {
0..70 => PixelSize::Octant, // U+1CD00 octants (Unicode 16)
70..80 => PixelSize::QuarterHeight, // also U+1CD00 octants
_ => PixelSize::ThirdHeight, // U+1FB00 sextants
};
```
`tui-big-text` warns about exactly this on all three variants ("depending on
how the used terminal renders characters, the generated text with this
PixelSize might look very strange"). Its remaining sizes — `Full`,
`HalfHeight`, `HalfWidth`, `Quadrant` — use only Block Elements
(U+2580..U+259F), which is universally supported, but none were reachable.
Switching fonts is not a workaround: `Octant`/`QuarterHeight` need U+1CD00,
added in Unicode 16 (Sept 2024), which essentially no shipping font has.
The underlying cause is the header height. `ui::render` gave the header
`Constraint::Length(6)` for a two-line banner, so each line got 3 rows — and
per `PixelSize::pixels_per_cell`, every size that fits two lines into 6 rows is
a legacy-computing one. The exotic glyphs were a consequence of the cramped
header, so fixing the glyphs means giving the banner the vertical room it needs.
# What changes are included in this PR?
- Header grows from `Constraint::Length(6)` to `Length(8)`, costing 2 rows
from the main view.
- Banner uses `PixelSize::Quadrant` (4 rows per line, Block Elements only).
The width-based `match` is dropped: `Quadrant` is 4 columns per character,
identical to the `Octant` it replaces, so narrow-terminal behaviour is
unchanged.
- Two regression tests rendering through a `TestBackend`:
- `banner_uses_only_widely_supported_block_element_glyphs` asserts every
emitted glyph falls in U+2580..U+259F.
- `banner_natural_height_matches_the_header_height` renders into an
over-tall buffer and pins the banner's natural height to the header's, so a
future `PixelSize` change can neither silently clip the banner nor leave the
header underfilled.
The banner now renders as:
```
▜▛▙ ▟ ▜▛▀▌ ▀
▐▌▐▌▝▀▙ ▝█▀ ▝▀▙ ▐▙▌ █ █ ▟▀▀ ▝█ ▟▀▙ █▀▙
▐▌▟▘▟▀█ █▗ ▟▀█ ▐▌▘ █ █ ▝▀▙ █ █ █ █ █
▀▀▘ ▝▀▝▘ ▝▘ ▝▀▝▘▀▀ ▝▀▝▘▀▀▘ ▝▀▘ ▝▀▘ ▀ ▀
▜▛▜▖ ▝█ ▝█ ▀ ▟
▐▙▟▘▝▀▙ █ █ ▝█ ▟▀▀ ▝█▀ ▝▀▙
▐▌▐▌▟▀█ █ █ █ ▝▀▙ █▗ ▟▀█
▀▀▀ ▝▀▝▘▝▀▘ ▝▀▘ ▝▀▘ ▀▀▘ ▝▘ ▝▀▝▘
```
Both new tests were confirmed to fail on the code they guard against, not
merely to pass on the fix: reverting to `ThirdHeight` fails the glyph test
(reporting the offending sextants) and the height test (6 rows vs 8), while an
over-tall `PixelSize::Full` passes the glyph test but is correctly caught by
the height test (15 rows vs 8).
`cargo test -p ballista-cli` passes (246 tests), and clippy is clean on both
feature combos CI runs for this crate (`cli,tui` and `web`).
# Are there any user-facing changes?
Yes, visually. The banner is legible instead of garbled, and the TUI header
is 2 rows taller, so the main view is 2 rows shorter. No public API changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
