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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
Levi Neuwirth 2026-08-07 18:22:40 +02:00
parent d8cf809b97
commit 4a26f0005e
No known key found for this signature in database
2 changed files with 155 additions and 2 deletions

View File

@ -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

View File

@ -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<T: mlua::FromLuaMulti>(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::<String>(&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::<String>(&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"
);
}