fix(statusline): harden narrow-band review edges
Document and pin the GPU built-in-only narrow-band clipping policy, including the intentional ability of a wide right group to hide the left identity. Guard the fixed UI face ordering used by binary search, preserve flattened provider tracebacks in *errors*, and rename the phase-one unavailable reason to cover missing layout contexts. Refresh the implementation verification record after the full gate suite. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
66765b3cbc
commit
1be7a30468
|
|
@ -118,9 +118,9 @@ next machine reads it the way you just did.
|
|||
`StatuslineSegments`; authoritative-empty/snapshot symmetry; and
|
||||
atomic GPU validation, face resolution, shaping, clipping, and
|
||||
cache invalidation. Acceptance 1-27 is implemented. Final gates:
|
||||
Clippy clean; 1,617 default + 1,791 CRDT library tests; 7 default +
|
||||
8 CRDT feature acceptance; 114 M4; 108 required GPU; one-invocation
|
||||
workspace sweep 2,715 passed across 78 suites (19 ignored,
|
||||
Clippy clean; 1,619 default + 1,793 CRDT library tests; 7 default +
|
||||
8 CRDT feature acceptance; 114 M4; 109 required GPU; one-invocation
|
||||
workspace sweep 2,718 passed across 78 suites (19 ignored,
|
||||
`basedpyright` filtered); `git diff --check` clean. This branch is
|
||||
awaiting review and **must not be described as merged**.
|
||||
- Roadmap: `docs/roadmap-2026-07.md` (ranked arcs). Position:
|
||||
|
|
|
|||
|
|
@ -8,6 +8,10 @@ and is fully gated; awaiting review, not merged.**
|
|||
Revision 3: closes review findings on authoritative-empty baseline retention
|
||||
and the TUI's protected-suffix clipping boundary.
|
||||
|
||||
The implementation review corrected the record for the GPU's
|
||||
built-in-only narrow-band case: stage 3 deliberately changes the legacy
|
||||
clipping edge and now pins that behavior with a headless regression test.
|
||||
|
||||
Revision 2: closes review findings on invalidation, terminal-control-safe
|
||||
grapheme painting, separator ownership, detached-frontend latches, and the
|
||||
unknown-LSP label. Revision 1 was the initial post-#124 architecture scout.
|
||||
|
|
@ -40,16 +44,17 @@ framed ownership boundary:
|
|||
`BufferSnapshot`.
|
||||
- The GPU consumes v18 atomically, resolves exact dynamic faces, clips
|
||||
provider runs without wrapping or displacing the protected suffix,
|
||||
and preserves its prior valid state on malformed input.
|
||||
deliberately right-pins over-wide built-in-only readouts, and
|
||||
preserves its prior valid state on malformed input.
|
||||
- `builtin/runtime/lsp.lua` registers the first pure right-side provider
|
||||
from its private attachment map; the Rust tracker exposes bounded
|
||||
`init`/`ready`/`degraded`/`crashed`/`stopped`/unknown labels.
|
||||
|
||||
The final gate run was sequential and clean: `cargo fmt --check`;
|
||||
workspace/all-target Clippy with `-D warnings`; 1,617 default and 1,791
|
||||
workspace/all-target Clippy with `-D warnings`; 1,619 default and 1,793
|
||||
CRDT library tests; 7 default and 8 CRDT stage-3 acceptance tests; 114
|
||||
M4 acceptance tests (3 ignored, `basedpyright` filtered); 108 required
|
||||
GPU tests; and the one-invocation workspace sweep (2,715 passed across
|
||||
M4 acceptance tests (3 ignored, `basedpyright` filtered); 109 required
|
||||
GPU tests; and the one-invocation workspace sweep (2,718 passed across
|
||||
78 suites, 19 ignored, `basedpyright` filtered). `git diff --check` was
|
||||
clean. No flaky rerun was needed.
|
||||
|
||||
|
|
@ -437,7 +442,7 @@ or no enabled providers is an O(1) fast path.
|
|||
|
||||
### Q#SL4 - Composition, order, separators, and narrow-window policy
|
||||
|
||||
Current built-ins remain protected:
|
||||
Current built-in positions remain anchored:
|
||||
|
||||
- **Left:** the frontend's current active/modified/buffer-identity group,
|
||||
with its existing edge padding, then custom left segments.
|
||||
|
|
@ -462,20 +467,25 @@ Priority means **survival priority when horizontal space is tight**:
|
|||
|
||||
- Left custom providers are ordered by `(priority descending,
|
||||
registration id ascending)`. Higher-priority items sit closest to the
|
||||
protected buffer identity. Overflow clips the low-priority tail.
|
||||
leading-edge buffer identity. Overflow clips the low-priority tail.
|
||||
- Right custom providers are displayed by `(priority ascending,
|
||||
registration id ascending)`, placing higher-priority items closest to
|
||||
the protected diagnostic/cursor/scroll suffix. The complete right run
|
||||
is right-aligned; overflow clips its low-priority left edge.
|
||||
- The protected built-in suffix is never discarded merely because a
|
||||
custom provider is long. If the built-in suffix itself cannot fit,
|
||||
each frontend retains today's behavior: the TUI drops the right group
|
||||
wholesale, while the GPU keeps its right edge fixed and clips its left
|
||||
edge. Custom-prefix clipping preserves the complete built-in suffix
|
||||
only when that suffix fits by itself.
|
||||
the TUI retains its legacy wholesale drop. The GPU deliberately
|
||||
changes its legacy narrow-band policy: before stage 3 it pinned the
|
||||
built-in group's left edge and clipped the right tail; stage 3 pins
|
||||
the right edge and clips the left so the readout tail survives.
|
||||
Custom-prefix clipping preserves the complete built-in suffix only
|
||||
when that suffix fits by itself.
|
||||
- The left group gets the space before the right group's measured
|
||||
origin and clips at the collision boundary. It never overwrites the
|
||||
right group.
|
||||
origin and clips at the collision boundary, without the legacy GPU's
|
||||
extra 10-pixel gap. It never overwrites the right group. This anchors
|
||||
buffer identity at the leading edge but does not guarantee its
|
||||
survival: an over-wide right group may consume all available left
|
||||
space.
|
||||
|
||||
This asymmetric visual ordering is intentional: priority determines
|
||||
what survives, not a generic ascending sort that would protect opposite
|
||||
|
|
@ -743,9 +753,14 @@ same belt as `StatusFacts`.
|
|||
- Right placement uses the full shaped width without clamping its
|
||||
origin to `TEXT_LEFT`: the run's right edge stays at the right pad,
|
||||
while a negative/left-of-surface origin clips low-priority custom
|
||||
prefixes and preserves the built-in tail. The left TextArea clips at
|
||||
the right group's actual origin. Existing geometry bounds still keep
|
||||
all glyphs inside the band.
|
||||
prefixes and preserves the built-in tail. This intentionally changes
|
||||
the legacy built-in-only narrow case, which anchored the readout at
|
||||
`TEXT_LEFT` and clipped its right tail.
|
||||
- The left TextArea clips at the right group's actual origin rather
|
||||
than retaining the legacy extra `STATUS_TEXT_PAD` gap. The right
|
||||
group therefore owns collision priority and may fully obscure the
|
||||
left buffer identity in an extremely narrow band. Existing geometry
|
||||
bounds still keep all glyphs inside the band.
|
||||
- `ThemeFacts` continues to invalidate both caches. FontFacts already
|
||||
re-metrics/re-shapes both status buffers; the new rich runs ride that
|
||||
path without a new font transaction.
|
||||
|
|
@ -845,8 +860,11 @@ TUI, producer, and daemon/wire behavior; protocol pins stay in
|
|||
`RenderState`/semantic frame paths, not direct helper-only formatting.
|
||||
|
||||
1. **Default preservation:** with no visible provider output, scratch
|
||||
TUI cells and GPU pixels are byte-identical to the pre-stage
|
||||
modeline/status band. The global TUI echo row is unchanged.
|
||||
TUI cells and ordinary non-overlapping GPU modeline/status-band
|
||||
pixels are byte-identical to the pre-stage rendering. The deliberate
|
||||
GPU narrow-band exception pins an over-wide built-in readout's right
|
||||
edge and clips its left edge; a built-in-only headless fixture pins
|
||||
that behavior. The global TUI echo row is unchanged.
|
||||
2. **Lua strict contract:** valid registration returns a handle and
|
||||
appears in `providers`; bad/unknown side, empty name, non-integer or
|
||||
out-of-range priority, non-function `fn`, non-modeline face
|
||||
|
|
|
|||
|
|
@ -4156,7 +4156,7 @@ impl State {
|
|||
|
||||
/// Compose the left group. Minibuffer, isearch, and transient
|
||||
/// messages suppress custom left segments; ordinary buffer identity
|
||||
/// stays protected at the leading edge.
|
||||
/// starts at the leading edge but may be fully clipped by the right group.
|
||||
fn compose_status_left_runs(&self) -> Vec<(String, Color)> {
|
||||
if let Some(minibuffer) = self.minibuffer.as_ref() {
|
||||
return vec![(
|
||||
|
|
@ -10007,6 +10007,64 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn built_in_only_overwide_readout_clips_left_and_keeps_its_right_tail_pinned() {
|
||||
let (narrow_width, wide_width, height) = (96, 500, 260);
|
||||
let Some(mut narrow) = headless_or_skip(narrow_width, height, "text") else {
|
||||
return;
|
||||
};
|
||||
let Some(mut wide) = headless_or_skip(wide_width, height, "text") else {
|
||||
return;
|
||||
};
|
||||
for state in [&mut narrow, &mut wide] {
|
||||
let buffer_id = BufferId::next();
|
||||
state.current_buffer_id = Some(buffer_id);
|
||||
state.status_facts = Some(status_facts(buffer_id, None));
|
||||
state.own_cursor = Some(OwnCursor { buffer_id, byte: 0 });
|
||||
}
|
||||
|
||||
let narrow_frame = narrow.render_offscreen();
|
||||
let wide_frame = wide.render_offscreen();
|
||||
assert!(
|
||||
narrow.statusline_segments.is_none() && wide.statusline_segments.is_none(),
|
||||
"fixture must exercise the built-in-only legacy surface"
|
||||
);
|
||||
let narrow_status_width = narrow
|
||||
.status_buffer
|
||||
.layout_runs()
|
||||
.map(|run| run.line_w)
|
||||
.fold(0.0_f32, f32::max);
|
||||
let wide_status_width = wide
|
||||
.status_buffer
|
||||
.layout_runs()
|
||||
.map(|run| run.line_w)
|
||||
.fold(0.0_f32, f32::max);
|
||||
assert!(
|
||||
(narrow_status_width - wide_status_width).abs() < 0.01,
|
||||
"surface width must not reshape the no-wrap readout"
|
||||
);
|
||||
assert!(
|
||||
narrow_width as f32 - STATUS_TEXT_PAD - narrow_status_width < 0.0,
|
||||
"fixture must force the built-in readout past the left edge"
|
||||
);
|
||||
assert!(
|
||||
wide_width as f32 - STATUS_TEXT_PAD - wide_status_width > 0.0,
|
||||
"comparison surface must fit the complete built-in readout"
|
||||
);
|
||||
|
||||
let band_top = text_area_bottom(height, narrow.fm).floor() as u32;
|
||||
let pinned_tail_width = 80;
|
||||
for y in band_top..height {
|
||||
for offset in 0..pinned_tail_width {
|
||||
assert_eq!(
|
||||
px_at(&narrow_frame, narrow_width, narrow_width - 1 - offset, y),
|
||||
px_at(&wide_frame, wide_width, wide_width - 1 - offset, y),
|
||||
"built-in readout tail moved at right-edge offset {offset}, y={y}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn overwide_status_runs_never_wrap_and_keep_the_suffix_pinned() {
|
||||
let (width, height) = (800, 300);
|
||||
|
|
|
|||
|
|
@ -5601,7 +5601,7 @@ fn log_hook_error(lua: &Lua, hook_name: &str, err: &crate::hook::HookCallbackErr
|
|||
/// this function owns only the repository-standard durable sink and window
|
||||
/// invalidation.
|
||||
pub(crate) fn log_statusline_provider_error(lua: &Lua, failure: &StatuslineProviderFailure) {
|
||||
let message = crate::statusline::sanitize_provider_text(&failure.message);
|
||||
let message = crate::statusline::sanitize_provider_error_text(&failure.message);
|
||||
let line = format!(
|
||||
"[statusline:{}] provider registered at {} failed for {:?}/{:?}/{:?}/active={}: {}\n",
|
||||
failure.provider_name,
|
||||
|
|
|
|||
|
|
@ -2381,6 +2381,14 @@ mod tests {
|
|||
state.core.borrow().active_window().buffer_id
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fixed_ui_face_inventory_is_strictly_sorted() {
|
||||
assert!(
|
||||
UI_FACES.windows(2).all(|pair| pair[0] < pair[1]),
|
||||
"theme_facts_msg uses binary_search; duplicates or unsorted insertions misclassify faces"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn line_numbers_emitted_on_toggle_then_suppressed() {
|
||||
// UX gutter (protocol v13): the daemon ships the per-window gutter
|
||||
|
|
|
|||
|
|
@ -155,8 +155,8 @@ pub struct StatuslineWindowSegments {
|
|||
/// Why phase 1 intentionally produced no message.
|
||||
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
|
||||
pub enum StatuslineNoMessageReason {
|
||||
/// The frontend/view no longer exists.
|
||||
FrontendUnavailable,
|
||||
/// The target frontend, view, or one of its layout windows no longer exists.
|
||||
ContextUnavailable,
|
||||
/// A layout window points at a buffer that has been removed.
|
||||
BufferUnavailable,
|
||||
/// The semantic frontend's daemon window does not match its declared
|
||||
|
|
@ -612,13 +612,13 @@ fn capture_target_contexts(
|
|||
let view = core
|
||||
.views
|
||||
.get(&frontend_id)
|
||||
.ok_or(StatuslineNoMessageReason::FrontendUnavailable)?;
|
||||
.ok_or(StatuslineNoMessageReason::ContextUnavailable)?;
|
||||
let mut contexts = Vec::new();
|
||||
for window_id in view.layout.iter_ids() {
|
||||
let window = core
|
||||
.windows
|
||||
.get(&window_id)
|
||||
.ok_or(StatuslineNoMessageReason::FrontendUnavailable)?;
|
||||
.ok_or(StatuslineNoMessageReason::ContextUnavailable)?;
|
||||
if buffers.get(window.buffer_id).is_err() {
|
||||
return Err(StatuslineNoMessageReason::BufferUnavailable);
|
||||
}
|
||||
|
|
@ -638,11 +638,11 @@ fn capture_target_contexts(
|
|||
let view = core
|
||||
.views
|
||||
.get(&frontend_id)
|
||||
.ok_or(StatuslineNoMessageReason::FrontendUnavailable)?;
|
||||
.ok_or(StatuslineNoMessageReason::ContextUnavailable)?;
|
||||
let window = core
|
||||
.windows
|
||||
.get(&view.active)
|
||||
.ok_or(StatuslineNoMessageReason::FrontendUnavailable)?;
|
||||
.ok_or(StatuslineNoMessageReason::ContextUnavailable)?;
|
||||
if buffers.get(window.buffer_id).is_err() {
|
||||
return Err(StatuslineNoMessageReason::BufferUnavailable);
|
||||
}
|
||||
|
|
@ -748,6 +748,15 @@ pub fn sanitize_provider_text(text: &str) -> String {
|
|||
output
|
||||
}
|
||||
|
||||
/// Flatten a provider failure into one durable `*errors*` entry while
|
||||
/// retaining multi-line traceback content.
|
||||
#[must_use]
|
||||
pub(crate) fn sanitize_provider_error_text(text: &str) -> String {
|
||||
text.chars()
|
||||
.map(|ch| if ch.is_control() { ' ' } else { ch })
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
|
@ -763,6 +772,48 @@ mod tests {
|
|||
(host, core, statusline)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_output_and_error_log_have_distinct_newline_policies() {
|
||||
let multiline = "boom\nstack\ttrace\r\0";
|
||||
assert_eq!(sanitize_provider_text(multiline), "boom");
|
||||
assert_eq!(
|
||||
sanitize_provider_error_text(multiline),
|
||||
"boom stack trace "
|
||||
);
|
||||
|
||||
let (host, core, registry) = harness();
|
||||
host.lua()
|
||||
.load(
|
||||
r"
|
||||
pmacs.statusline.register {
|
||||
name='trace', side='left',
|
||||
fn=function() error('boom\nstack detail') end,
|
||||
}
|
||||
",
|
||||
)
|
||||
.exec()
|
||||
.unwrap();
|
||||
let evaluation = evaluate_statusline(
|
||||
host.lua(),
|
||||
&core,
|
||||
®istry,
|
||||
StatuslineEvaluationTarget::Grid {
|
||||
frontend_id: FrontendId::LOCAL,
|
||||
},
|
||||
);
|
||||
assert_eq!(evaluation.new_failures.len(), 1);
|
||||
let errors = host.errors_buffer_text();
|
||||
assert!(
|
||||
errors.contains("boom stack detail") && errors.contains("stack traceback"),
|
||||
"flattened provider traceback must retain every line: {errors:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
errors.lines().count(),
|
||||
1,
|
||||
"one failure run must remain one durable error entry"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn registry_epochs_track_layout_and_distinct_enabled_faces() {
|
||||
let lua = Lua::new();
|
||||
|
|
|
|||
Loading…
Reference in New Issue