fix(tui): keep unselected list rows on the cursor column; trim tests
Follow-up to the salvaged #113760 hunk. The pick used ``indent`` as the lead for unselected rows, which is four spaces in the command palette — every unselected palette row shifted two columns right of the ``❯`` row. The lead is now always the two-column cursor cell; ``indent`` stays the continuation indent for wrapped rows, as before. Tests trimmed to two invariants on the real renderer: the reporter's 54-char model id keeps ``❯`` and the label on one row and keeps its indent when unselected (every row within the panel body); palette rows align on the cursor column, wrap with the 4-space indent, and an unbreakable label that fills the body keeps the cursor on its row. Both are red on origin/main.
This commit is contained in:
teknium1
@@ -631,7 +631,10 @@ class CLITuiMixin:
|
||||
panel.blank()
|
||||
for idx in range(scroll_offset, min(scroll_offset + visible, len(labels))):
|
||||
style = 'class:clarify-selected' if idx == selected else 'class:clarify-choice'
|
||||
lead = '❯ ' if idx == selected else indent
|
||||
# The cursor cell is always two columns wide, so unselected rows get two spaces
|
||||
# regardless of ``indent`` (the palette's continuation indent is four) — otherwise
|
||||
# the selected label starts two columns left of its neighbours.
|
||||
lead = '❯ ' if idx == selected else ' '
|
||||
for wrapped in _prefix_wrapped_rows(
|
||||
_wrap_panel_text, labels[idx], label_width, lead, indent
|
||||
):
|
||||
|
||||
@@ -1,139 +1,60 @@
|
||||
"""Regression: long labels mis-rendered in the scrollable list panel.
|
||||
"""Invariants for ``CLITuiMixin._render_scroll_list_panel`` (the ``/model`` picker's model
|
||||
stage and the command palette) with labels long enough to fill the panel body.
|
||||
|
||||
Covers ``CLITuiMixin._render_scroll_list_panel`` (the ``/model`` picker's model
|
||||
stage and the command palette) and its ``_prefix_wrapped_rows`` helper.
|
||||
|
||||
Symptom, with a provider whose model ids are long — e.g. MLX Core's
|
||||
``peculiar-ragdoll/Cyber-Tiel-Coder-35B-A3B-MLX-oQ4e-MTP`` (54 chars):
|
||||
|
||||
* moving the cursor onto such a row left ``❯`` alone on its own line, with the
|
||||
model name pushed to the row below; and
|
||||
* unselected long rows lost their leading indent and rendered flush against the
|
||||
panel border, out of alignment with every other row.
|
||||
|
||||
Cause: the 2-char cursor/indent prefix was concatenated onto the label *before*
|
||||
wrapping, so it was charged against the label's width budget, and the wrapper's
|
||||
whitespace trimming removed the leading indent.
|
||||
The prefix (cursor or indent) must never be charged against the label's wrap budget: a 54-char
|
||||
model id such as MLX Core's ``peculiar-ragdoll/Cyber-Tiel-Coder-35B-A3B-MLX-oQ4e-MTP`` used to
|
||||
strand ``❯`` on a row of its own when selected, and lose its indent when not.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from cli import _panel_box_width
|
||||
from hermes_cli.cli_tui_mixin import CLITuiMixin, _prefix_wrapped_rows
|
||||
from hermes_cli.cli_tui_mixin import CLITuiMixin
|
||||
|
||||
|
||||
# A real long model id from the MLX Core (LAN) provider.
|
||||
LONG_LABEL = "peculiar-ragdoll/Cyber-Tiel-Coder-35B-A3B-MLX-oQ4e-MTP"
|
||||
SHORT_LABELS = ["MiniMax-M3", "MiniMax-M2.7", "MiniMax-M2.1"]
|
||||
PICKER_TITLE = "⚙ Model Picker — MLX Core (LAN)"
|
||||
HINT = "Select a model (7 available) — type to filter"
|
||||
|
||||
|
||||
class _Host(CLITuiMixin):
|
||||
"""Minimal host: the renderer only touches `state` + module helpers."""
|
||||
"""Minimal host: the renderer only touches ``state`` + module helpers."""
|
||||
|
||||
|
||||
def _panel_rows(fragments):
|
||||
"""Reconstruct the padded body text of each bordered panel row."""
|
||||
def _label_rows(fragments, hint):
|
||||
text = "".join(t for _style, t in fragments)
|
||||
rows = []
|
||||
for line in text.split("\n"):
|
||||
if line.startswith("│") and line.rstrip().endswith("│"):
|
||||
rows.append(line[2:-2])
|
||||
return rows
|
||||
rows = [line[2:-2] for line in text.split("\n") if line.startswith("│ ") and line.endswith(" │")]
|
||||
return [r for r in rows if r.strip() and not r.startswith(hint)]
|
||||
|
||||
|
||||
def _label_rows(fragments):
|
||||
"""Panel rows that carry a list label (skip blanks and the hint row)."""
|
||||
return [r for r in _panel_rows(fragments) if r.strip() and not r.startswith("Select a model")]
|
||||
def _render(labels, selected, *, indent=" ", min_width=46, max_width=84, title=PICKER_TITLE, hint=HINT):
|
||||
frags = _Host()._render_scroll_list_panel(
|
||||
{"selected": selected}, title, hint, labels, min_width=min_width, max_width=max_width, indent=indent)
|
||||
return _label_rows(frags, hint)
|
||||
|
||||
|
||||
def _render(labels, selected=0, indent=" ", title=PICKER_TITLE, hint="Select a model"):
|
||||
host = _Host()
|
||||
return host._render_scroll_list_panel(
|
||||
{"selected": selected}, title, hint, labels,
|
||||
min_width=46, max_width=84, indent=indent,
|
||||
)
|
||||
def test_long_model_id_stays_on_the_cursor_row_and_keeps_its_indent():
|
||||
labels = ["root4k/Huihui-Qwen3.6-35B-A3B-abliterated-oQ4e-mtp", LONG_LABEL, "← Back", "Cancel"]
|
||||
body = _panel_box_width(PICKER_TITLE, [HINT] + labels, min_width=46, max_width=84) - 2
|
||||
|
||||
selected = [r.rstrip() for r in _render(labels, 1)]
|
||||
assert selected[1] == f"❯ {LONG_LABEL}", selected # cursor and full label on ONE row
|
||||
unselected = [r.rstrip() for r in _render(labels, 0)]
|
||||
assert unselected[1] == f" {LONG_LABEL}", unselected # indent kept, aligned with neighbours
|
||||
assert unselected[0] == f"❯ {labels[0]}"
|
||||
for rows in (selected, unselected):
|
||||
assert len(rows) == len(labels)
|
||||
assert all(len(r) <= body for r in rows), rows
|
||||
|
||||
|
||||
def _box_width(title, labels, hint):
|
||||
return _panel_box_width(title, [hint] + labels, min_width=46, max_width=84)
|
||||
def test_palette_rows_align_on_the_cursor_cell_and_wrap_with_its_indent():
|
||||
hint = "Type to filter 3 commands — ↑/↓ then Enter inserts, Esc cancels"
|
||||
long_label = "/very-long-command-name — " + "x" * 60
|
||||
labels = ["/model — Switch the active model", long_label, "/help — Help"]
|
||||
rows = [r.rstrip() for r in _render(labels, 1, indent=" ", min_width=50, max_width=90,
|
||||
title="⚙ Command Palette", hint=hint)]
|
||||
assert rows[0] == f" {labels[0]}" # unselected: two-column lead, same column as the cursor row
|
||||
assert rows[1].startswith("❯ /very-long-command-name")
|
||||
assert rows[2] == " " + "x" * 60 # continuation row carries the palette's 4-space indent
|
||||
assert rows[3] == f" {labels[2]}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helper contract
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestPrefixWrappedRows:
|
||||
def test_prefix_is_not_charged_against_the_wrap_width(self):
|
||||
wrap = lambda text, width: [text[:width], text[width:]] if len(text) > width else [text]
|
||||
rows = _prefix_wrapped_rows(wrap, "abcdefgh", 8, "❯ ", " ")
|
||||
assert rows == ["❯ abcdefgh"]
|
||||
|
||||
def test_continuation_rows_use_the_indent(self):
|
||||
# A wrapper that splits into two rows of 4 chars.
|
||||
wrap = lambda text, width: [text[i:i + 4] for i in range(0, len(text), 4)]
|
||||
rows = _prefix_wrapped_rows(wrap, "abcdefgh", 4, "❯ ", " ")
|
||||
assert rows == ["❯ abcd", " efgh"]
|
||||
|
||||
def test_first_prefix_only_applies_to_the_first_row(self):
|
||||
wrap = lambda text, width: ["a", "b", "c"]
|
||||
rows = _prefix_wrapped_rows(wrap, "x", 4, "❯ ", " ")
|
||||
assert rows == ["❯ a", " b", " c"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Real renderer: long labels
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestLongLabelRendering:
|
||||
def test_selected_long_label_keeps_cursor_and_name_on_one_row(self):
|
||||
rows = _label_rows(_render([LONG_LABEL], selected=0))
|
||||
assert len(rows) == 1, f"label split across rows: {rows}"
|
||||
assert rows[0].startswith("❯ ")
|
||||
assert rows[0][2:].strip() == LONG_LABEL
|
||||
|
||||
def test_unselected_long_label_keeps_its_indent(self):
|
||||
labels = [SHORT_LABELS[0], LONG_LABEL, SHORT_LABELS[1]]
|
||||
rows = _label_rows(_render(labels, selected=0))
|
||||
long_row = next(r for r in rows if LONG_LABEL in r)
|
||||
assert long_row.startswith(" "), repr(long_row)
|
||||
assert not long_row.startswith("❯"), repr(long_row)
|
||||
|
||||
@pytest.mark.parametrize("selected", [0, 1, 2])
|
||||
def test_every_row_keeps_a_two_column_lead(self, selected):
|
||||
labels = [SHORT_LABELS[0], LONG_LABEL, SHORT_LABELS[1]]
|
||||
for row in _label_rows(_render(labels, selected=selected)):
|
||||
assert row[:2] in ("❯ ", " "), repr(row)
|
||||
|
||||
def test_no_row_exceeds_the_panel_body(self):
|
||||
labels = [LONG_LABEL] + SHORT_LABELS + ["← Back", "Cancel"]
|
||||
body = _box_width(PICKER_TITLE, labels, "Select a model") - 2
|
||||
for selected in range(len(labels)):
|
||||
for row in _label_rows(_render(labels, selected=selected)):
|
||||
assert len(row) <= body, repr(row)
|
||||
|
||||
|
||||
class TestShortLabelsUnchanged:
|
||||
def test_short_labels_render_with_two_column_lead(self):
|
||||
# Rows are ljust-padded to the panel body width by _Panel.row.
|
||||
rows = [r.rstrip() for r in
|
||||
_label_rows(_render(SHORT_LABELS + ["← Back", "Cancel"], selected=1))]
|
||||
assert rows[0] == " MiniMax-M3"
|
||||
assert rows[1] == "❯ MiniMax-M2.7"
|
||||
assert rows[-2] == " ← Back"
|
||||
assert rows[-1] == " Cancel"
|
||||
|
||||
|
||||
class TestPaletteIndent:
|
||||
"""The palette uses a 4-space indent instead of the picker's 2."""
|
||||
|
||||
def test_long_palette_label_keeps_cursor_and_text_on_one_row(self):
|
||||
label = "a" * 60
|
||||
rows = _label_rows(_render([label], selected=0, indent=" "))
|
||||
assert len(rows) == 1, rows
|
||||
assert rows[0].startswith("❯ ")
|
||||
|
||||
def test_palette_rows_respect_the_wider_indent(self):
|
||||
label = "alpha beta gamma delta epsilon zeta eta theta iota kappa lambda mu"
|
||||
rows = _label_rows(_render([label], selected=0, indent=" "))
|
||||
for row in rows:
|
||||
assert row.startswith(("❯ ", " ")), repr(row)
|
||||
token = "/" + "y" * 69 # one unbreakable token that fills the body: cursor must stay on its row
|
||||
rows = [r.rstrip() for r in _render([token, "/help — Help"], 0, indent=" ", min_width=50,
|
||||
max_width=90, title="⚙ Command Palette", hint=hint)]
|
||||
assert rows == [f"❯ {token}", " /help — Help"], rows
|
||||
|
||||
Reference in New Issue
Block a user