fix(find-file): review round 1 -- name the real test, pin two gaps

Three of the five review findings land here; the other two are recorded
as named deferrals in the framing on the dired branch.

Finding 1: the command comment cited "acc4", a name from a draft scheme
that no test carries. It now names the real test, and the comment splits
the shadowing consequence into the two cases that actually exist -- a new
bare name that matches an entry (shadowed) versus one that matches
nothing (creates normally) -- each pointing at its test.

Finding 2: the everyday new-file flow had no test. Typing a bare name
that is not a subsequence of any entry is the path users hit first, and
the only route combining free text with a relative join; every existing
new-file test used a name containing a separator.
find_file_bare_new_name_creates_in_the_root covers it, asserting the
parent is the prompt's root so the join itself is pinned.

Finding 3: the failure arm was never exercised, and as the review noted,
deleting the pcall would have passed the whole suite. Accepting a
directory candidate reaches display_file, whose load fails because
File::open on a directory succeeds and the read returns EISDIR;
find_file_accepting_a_directory_reports_instead_of_raising pins that this
surfaces as the command's status message, leaves the active buffer alone,
and closes the prompt. Verified by manual revert: with the pcall replaced
by a direct call, that test and only that test fails. scripts/bite could
not isolate it, since the guard and its test have no separating commit.

Finding 4 is documented at the command rather than left implicit:
accepting on empty input opens the first-sorted candidate, because
fuzzy_score returns Some(0) for an empty needle and filter_and_sort
breaks the tie lexicographically, so dotfiles lead and a directory can
lead. M-x and switch-buffer share the mechanism, so it is inherited
rather than introduced, and it is listed in the framing beside the
accept-semantics change that would close it.
This commit is contained in:
Levi Neuwirth 2026-07-25 11:33:46 -04:00
parent 4a2aa92510
commit 0b0d5acd81
2 changed files with 96 additions and 4 deletions

View File

@ -637,10 +637,23 @@ cmd { name = "editor.switch-buffer",
-- are basenames and the filter is a subsequence match, is exactly
-- when the input contains a `/`. That makes the deeper-path case work
-- (`sub/inner.txt` matches no basename, so it arrives verbatim) and
-- leaves one documented hole: typing a NEW bare name that happens to
-- be a subsequence of an existing entry opens the existing file
-- instead of creating the new one. `acc4` pins that as a known
-- behavior rather than letting it be an accident.
-- leaves TWO documented consequences, each pinned by a test rather
-- than left to be rediscovered:
--
-- (a) typing a NEW bare name that happens to be a subsequence of an
-- existing entry opens the existing file instead of creating the
-- new one --- `find_file_selected_candidate_shadows_typed_text`.
-- A new bare name that matches nothing is unaffected and creates
-- normally (`find_file_bare_new_name_creates_in_the_root`).
-- (b) accepting on EMPTY input opens the first candidate in sort
-- order. `fuzzy_score` returns `Some(0)` for an empty needle, so
-- everything ties and `filter_and_sort` falls back to
-- lexicographic order --- which puts dotfiles first, and can put
-- a DIRECTORY first, in which case the open fails and reports.
-- This is the same mechanism `M-x` and `switch-buffer` already
-- have, so it is inherited rather than introduced; it is recorded
-- as decided, not overlooked, and listed in the framing's
-- deferrals beside the accept-semantics fix that would close it.
--
-- The root is the active buffer's directory, or the process cwd when the
-- buffer has no backing path (`source_root` defaults to "." Rust-side,

View File

@ -194,6 +194,85 @@ fn find_file_nonexistent_path_creates_a_new_file_buffer() {
);
}
/// The everyday new-file flow: a BARE name, no separator, matching no
/// existing entry. The candidate list empties on its own, so the typed
/// text arrives and joins onto the root. This is the path users hit
/// first, and it is the only route through `find_file_resolve` that
/// combines free text with a relative join.
#[test]
fn find_file_bare_new_name_creates_in_the_root() {
let td = tempfile::tempdir().expect("tempdir");
let fresh = td.path().join("zzz.txt");
let mut s = editor_in(td.path());
open_prompt(&mut s);
// "zzz.txt" is not a subsequence of "anchor.txt" (no 'z' in it), so
// nothing survives the filter and the typed name is what accepts.
type_str(&mut s, "zzz.txt");
assert!(
candidates(&s).is_empty(),
"fixture premise: a bare non-matching name must empty the list; got {:?}",
candidates(&s)
);
press(&mut s, KeyCode::Enter);
let path = active_path(&s).expect("a buffer must be bound to the new path");
assert_eq!(
std::path::Path::new(&path).parent(),
Some(td.path()),
"a bare name must join onto the prompt's root; got {path}"
);
assert!(
path.ends_with("zzz.txt"),
"the buffer must carry the typed name; got {path}"
);
let len: usize = eval(&s, "return pmacs.window.buffer():len()");
assert_eq!(len, 0, "a new-file buffer starts empty");
assert!(!fresh.exists(), "nothing is written to disk until save");
}
/// The failure arm. Accepting a DIRECTORY candidate reaches
/// `display_file`, whose load fails (opening a directory succeeds, the
/// read does not), and the command's `pcall` must turn that into a
/// status message rather than letting the error escape mid-dispatch.
/// Without the `pcall` this test fails, which is the point --- the
/// guard is pinned through the real accept path, not asserted directly.
#[test]
fn find_file_accepting_a_directory_reports_instead_of_raising() {
let td = tempfile::tempdir().expect("tempdir");
std::fs::create_dir(td.path().join("sub")).expect("mkdir");
let mut s = editor_in(td.path());
let before = active_path(&s).expect("the anchor must be open");
open_prompt(&mut s);
// Only the directory matches: "anchor.txt" contains no 's'.
type_str(&mut s, "sub");
assert_eq!(
candidates(&s),
vec!["sub".to_string()],
"fixture premise: the directory must be the sole candidate"
);
press(&mut s, KeyCode::Enter);
let line = status(&s);
assert!(
line.starts_with("find-file: "),
"the failure must surface as this command's status message; got {line:?}"
);
assert_eq!(
active_path(&s).as_deref(),
Some(before.as_str()),
"a failed open must leave the active buffer alone"
);
assert!(
!eval::<bool>(&s, "return pmacs.minibuffer.is_active()"),
"the prompt must have closed even though the open failed"
);
}
/// 0d --- with no backing path, the prompt roots at the process cwd
/// (`source_root` is omitted, and the Rust side defaults to ".").
/// The test crate's cwd is the crate root, so `Cargo.toml` is a