From 4a26f0005ee85134467d0a44a736d44a4d54409a Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 7 Aug 2026 18:22:40 +0200 Subject: [PATCH] fix(linewrap): the toggle wrote the global layer, not the buffer ui.line-wrap is buffer-local, and the toggle was built from pmacs.config.get(name) and pmacs.config.set(name, value) --- both of which address the GLOBAL layer. The registry's buffer-local surface is get(name, buf) and set_local(buf, name, value), and the command used neither. The result was wrong in both directions at once, which is why it needed two witnesses rather than one. In a buffer pinned to truncate, the toggle would read "wrap" from the global layer, decide the next mode is truncate, and leave that buffer exactly as it was --- while writing truncate globally and flipping every buffer that had no override of its own. The command that changes nothing here and everything elsewhere. Now resolves the buffer ONCE and uses it for both calls. Once matters: resolving twice would be a narrower version of the same bug, since the active buffer can change between two calls. Two witnesses in a new acceptance suite, and both bite against the code review rejected --- restoring the global-layer toggle fails both while the default and enum tests keep passing, which is exactly how it shipped. the_toggle_moves_this_buffer_and_leaves_the_other_alone covers the leak outward: a second buffer and the global layer must be untouched. a_pinned_buffer_toggles_from_its_own_value covers the miss inward: a buffer whose value differs from global must toggle from ITS value. Note on the second buffer: there is no Lua buffer-switch, so "the other" is a buffer that exists but is not shown. That is the case that matters anyway --- a global write reaches every buffer without an override, shown or not. Gates: fmt, workspace clippy -D warnings, diff --check, --lib 1916/0, line_wrap_acceptance 4/4. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- builtin/runtime/linewrap.lua | 18 ++++- tests/line_wrap_acceptance.rs | 139 ++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 tests/line_wrap_acceptance.rs diff --git a/builtin/runtime/linewrap.lua b/builtin/runtime/linewrap.lua index 1c68d51..39aae65 100644 --- a/builtin/runtime/linewrap.lua +++ b/builtin/runtime/linewrap.lua @@ -43,13 +43,27 @@ pmacs.config.define { -- rather than left for a user to discover, and it is why `truncate` is -- not the default despite being the TUI's historical behavior. +-- Buffer-local on BOTH sides, and the pairing matters. +-- +-- `pmacs.config.get(name)` reads the global chain and +-- `pmacs.config.set(name, ...)` writes the global layer, so a toggle +-- built from those two would be wrong in the case the setting exists +-- for: a buffer pinned to `truncate` would report the GLOBAL value, +-- flip the GLOBAL value, and leave that buffer exactly as it was --- +-- while silently changing every buffer that had not been pinned. +-- +-- So: read with `get(name, buf)`, write with `set_local(buf, ...)`, +-- and resolve the buffer ONCE for both. Resolving twice would be a +-- narrower version of the same bug, since the active buffer can change +-- between two calls. pmacs.command.define { name = "ui.toggle-line-wrap", description = "Toggle line wrapping for the current buffer", fn = function() - local current = pmacs.config.get("ui.line-wrap") + local buf = pmacs.window.buffer() + local current = pmacs.config.get("ui.line-wrap", buf) local next_mode = current == "wrap" and "truncate" or "wrap" - pmacs.config.set("ui.line-wrap", next_mode) + pmacs.config.set_local(buf, "ui.line-wrap", next_mode) if next_mode == "truncate" then pmacs.editor.set_status("line wrap off — text past the edge is unreachable until horizontal scrolling lands") else diff --git a/tests/line_wrap_acceptance.rs b/tests/line_wrap_acceptance.rs new file mode 100644 index 0000000..4c519c0 --- /dev/null +++ b/tests/line_wrap_acceptance.rs @@ -0,0 +1,139 @@ +//! Line-wrap acceptance (long lines, `docs/long-lines-framing.md`). +//! +//! `ui.line-wrap` is **buffer-local** (Q#LL2), and buffer-local is the +//! whole point rather than a nicety: prose wants wrapping and a log file +//! usually does not, in the same session. +//! +//! Which makes the toggle command's failure mode specific and invisible +//! in any single-buffer test. `pmacs.config.get(name)` reads the global +//! chain and `pmacs.config.set(name, ...)` writes the global layer, so a +//! toggle built from that pair reports and flips the **global** value. +//! In a buffer pinned to `truncate` it would leave that buffer exactly +//! as it was while silently changing every buffer that had not been +//! pinned — the opposite of what the user asked for, in both directions +//! at once. +//! +//! So these tests use a second buffer, and a buffer pinned against +//! the global layer, to make both halves of that failure visible. + +use std::path::Path; + +use pmacs::bootstrap::BootstrapRoots; +use pmacs::editor::EditorState; + +fn session(name: &str) -> EditorState { + let base = Path::new(env!("CARGO_TARGET_TMPDIR")) + .join("line-wrap") + .join(name); + let _ = std::fs::remove_dir_all(&base); + let roots = BootstrapRoots::isolated_under(&base); + for (_, dir) in roots.child_env() { + std::fs::create_dir_all(&dir).expect("create controlled root"); + } + let state = EditorState::new_with_roots(&roots); + state.install_state_dirs(); + state +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +/// The resolved mode for the buffer currently in the window. +fn mode_here(s: &EditorState) -> String { + eval( + s, + "return pmacs.config.get('ui.line-wrap', pmacs.window.buffer())", + ) +} + +/// The global layer's value, which is *not* what a buffer necessarily +/// resolves to. +fn mode_global(s: &EditorState) -> String { + eval(s, "return pmacs.config.get('ui.line-wrap')") +} + +#[test] +fn the_default_is_wrap() { + let s = session("default"); + assert_eq!(mode_global(&s), "wrap"); + assert_eq!( + mode_here(&s), + "wrap", + "with no buffer-local override, a buffer resolves to the global default" + ); +} + +#[test] +fn only_wrap_and_truncate_are_accepted() { + let s = session("enum"); + let err: bool = eval( + &s, + "local ok = pcall(pmacs.config.set, 'ui.line-wrap', 'sideways'); return not ok", + ); + assert!( + err, + "a closed choice set makes an unknown mode impossible rather than handled" + ); +} + +/// The finding this suite exists for: the toggle must move the buffer +/// it is invoked in, and **only** that buffer. +/// +/// There is no Lua buffer-switch, so "the other buffer" is a second +/// buffer that exists but is not shown — which is the case that matters +/// anyway: a global write reaches every buffer without an override of +/// its own, shown or not. +#[test] +fn the_toggle_moves_this_buffer_and_leaves_the_other_alone() { + let s = session("two_buffers"); + exec(&s, "OTHER = pmacs.buffer.create('other')"); + assert_eq!( + eval::(&s, "return pmacs.config.get('ui.line-wrap', OTHER)"), + "wrap", + "precondition: the second buffer starts at the default" + ); + + exec(&s, "pmacs.command.invoke('ui.toggle-line-wrap')"); + assert_eq!(mode_here(&s), "truncate", "the invoking buffer moved"); + + assert_eq!( + eval::(&s, "return pmacs.config.get('ui.line-wrap', OTHER)"), + "wrap", + "toggling in one buffer must not change another" + ); + assert_eq!( + mode_global(&s), + "wrap", + "a buffer-local toggle must not write the global layer — doing so \ + would change every buffer with no override of its own" + ); +} + +/// A buffer pinned to `truncate` must toggle back to `wrap`, reading its +/// own value rather than the global one. +/// +/// This is the half a global-read toggle gets wrong even if its write +/// were harmless: it would see `wrap` globally, decide the next mode is +/// `truncate`, and leave the pinned buffer exactly as it was. +#[test] +fn a_pinned_buffer_toggles_from_its_own_value() { + let s = session("pinned"); + exec( + &s, + "pmacs.config.set_local(pmacs.window.buffer(), 'ui.line-wrap', 'truncate')", + ); + assert_eq!(mode_here(&s), "truncate"); + assert_eq!(mode_global(&s), "wrap", "precondition: the layers differ"); + + exec(&s, "pmacs.command.invoke('ui.toggle-line-wrap')"); + assert_eq!( + mode_here(&s), + "wrap", + "the toggle read this buffer's value, not the global one" + ); +}