Skip to content

Commit cb30e50

Browse files
claudesinelaw
authored andcommitted
flash: lift conceal-mode gate so labels actually substitute glyphs
Side-by-side comparison with neovim+flash.nvim revealed that fresh's flash labels were styled (magenta cell) but still showed the *original character* underneath, where flash.nvim correctly shows the assigned label letter. Root cause: addConceal calls were landing in state but the renderer's apply_conceal_ranges was gated on `is_compose && !state.conceals.is_empty()`, so source-mode buffers (the common case for flash) never saw the substitution. Lifting the gate is safe: every plugin that adds source-mode conceals already self-checks the buffer's view mode (e.g. markdown_compose's `isComposing` helper), so plugins that never intend to substitute in source mode keep their existing behaviour. flash and any other plugin that legitimately wants overlay-style cell substitution in source mode now gets it. Why no test caught this: The existing e2e tests assert on cursor_position() after pressing a label. The labeler's logic was correct — the right label letter was assigned, the keypress dispatched correctly, the cursor landed at the right byte. None of the assertions touched the *rendered glyph at the label position*, so the visual bug slipped through. Adds flash_label_substitutes_rendered_glyph: drives flash to a labelled state and asserts on `screen_to_string()` that the cell right after at least one match has been substituted with a pool letter (so `hsllo` etc. instead of plain `hello`). Also asserts that fewer than N plain occurrences of the original word survive, catching the case where conceal silently no-ops. Comparison with flash.nvim now matches: same labels, same positions, same letters substituted. Verified by tmux side-by-side run with identical sample.rs file and identical cursor position. Tests: 6 flash + 46 vi_mode + 41 markdown_compose e2e tests all pass. Showcase GIF regenerated.
1 parent ea3ec22 commit cb30e50

3 files changed

Lines changed: 88 additions & 4 deletions

File tree

crates/fresh-editor/src/view/ui/split_rendering/view_data.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,16 @@ pub(super) fn build_view_data(
9494
}
9595
}
9696

97-
// Apply conceal ranges - filter/replace tokens that fall within concealed
98-
// byte ranges. Only apply in Compose mode; Source mode shows the raw
99-
// markdown syntax.
100-
if is_compose && !state.conceals.is_empty() {
97+
// Apply conceal ranges — filter or replace tokens that fall
98+
// within concealed byte ranges. This used to be gated on
99+
// `is_compose` so markdown source mode would always show raw
100+
// syntax, but the gate was redundant: every plugin that adds
101+
// source-mode conceals already self-checks the buffer's view
102+
// mode (see e.g. `markdown_compose.ts`'s `isComposing`). Other
103+
// plugins (flash) legitimately want overlay-style cell
104+
// substitution in source mode and were broken by the gate —
105+
// their `addConceal` calls landed in state but never rendered.
106+
if !state.conceals.is_empty() {
101107
let viewport_end = tokens
102108
.iter()
103109
.filter_map(|t| t.source_offset)

crates/fresh-editor/tests/e2e/flash.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,84 @@ fn flash_backspace_shrinks_pattern() {
193193
);
194194
}
195195

196+
/// Regression for the silent-conceal bug, 2026-04: flash relies on
197+
/// `addConceal` to substitute the next-char glyph with the label
198+
/// letter (overlay-style rendering, no layout shift). An earlier
199+
/// version of fresh's renderer gated `apply_conceal_ranges` on
200+
/// Compose mode only, so flash's conceal calls landed in state but
201+
/// never reached the rendered buffer — labels appeared on screen as
202+
/// the original character with the magenta style applied, not as
203+
/// the assigned label letter. Cursor-position assertions still
204+
/// passed (the labeler logic was correct), so no existing test
205+
/// caught it.
206+
///
207+
/// This test asserts the rendered glyph itself: at the screen
208+
/// position right after the first `s` match in the buffer, the
209+
/// rendered cell must contain the label letter `a`, not the
210+
/// original `e`.
211+
#[test]
212+
fn flash_label_substitutes_rendered_glyph() {
213+
// Same buffer shape as `flash_jumps_to_label` so the harness
214+
// setup that's already known to work doesn't surprise us.
215+
let (mut harness, _temp) = flash_harness(120, 24);
216+
let fixture = TestFixture::new("test.txt", "hello world\nhello there\nhello again\n").unwrap();
217+
harness.open_file(&fixture.path).unwrap();
218+
harness.render().unwrap();
219+
220+
arm_flash(&mut harness);
221+
// Pattern `h` — three matches at the start of each line. With
222+
// cursor at byte 0, the labeler assigns labels in distance order
223+
// from "asdfghjkl..." minus the next-char skip set. The next
224+
// char after each `h` is `e` (in "hello"), so the skip set is
225+
// {e}. Available pool: a, s, d, f, g, h, j, k, l, ...
226+
// Three matches → labels a, s, d.
227+
type_pattern(&mut harness, "h");
228+
harness.render().unwrap();
229+
230+
let screen = harness.screen_to_string();
231+
// The labels overlay-substitute the next-char glyph (the `e`
232+
// after each `h`). The literal label letters depend on the
233+
// labeler's stability rule (which carries empty-pattern mode's
234+
// labels through the first-character transition), so we don't
235+
// hard-code which letter lands where. What we assert is the
236+
// *substitution itself*: at every "hello" occurrence the `e`
237+
// immediately after the matched `h` must be replaced by SOME
238+
// label letter from the pool. If conceal isn't applied, the
239+
// original `hello` text comes through unchanged.
240+
let pool: &[char] = &[
241+
'a', 's', 'd', 'f', 'g', 'h', 'j', 'k', 'l', 'q', 'w', 'r', 't', 'y', 'u', 'i', 'o', 'p',
242+
'z', 'x', 'c', 'v', 'b', 'n', 'm',
243+
];
244+
let mut substituted_count = 0;
245+
for c in pool {
246+
let needle: String = format!("h{}llo", c);
247+
if screen.contains(&needle) {
248+
substituted_count += screen.matches(&needle).count();
249+
}
250+
}
251+
assert!(
252+
substituted_count >= 1,
253+
"expected at least one match to render with the next-char \
254+
`e` replaced by a pool label letter (e.g. `hsllo`, `hallo`, …) \
255+
— that's flash's overlay-style cell substitution. None \
256+
seen, so addConceal didn't paint. Screen:\n{}",
257+
screen,
258+
);
259+
// The original glyph `hello` must NOT survive at the labelled
260+
// positions. We can't easily count "labelled occurrences" from
261+
// the screen alone, but we can check there are FEWER plain
262+
// `hello`s than there are matches (3): if none were
263+
// substituted, all three would still read `hello`.
264+
let plain_hello = screen.matches("hello").count();
265+
assert!(
266+
plain_hello < 3,
267+
"expected the substitution to remove at least one plain \
268+
`hello`, but {} remain — conceal didn't apply. Screen:\n{}",
269+
plain_hello,
270+
screen,
271+
);
272+
}
273+
196274
#[test]
197275
fn flash_jumps_across_splits() {
198276
// Two vertical splits, each with a different buffer that contains
303 Bytes
Loading

0 commit comments

Comments
 (0)