fix(lsp): a `/` search boundary no longer disables the LaTeX marker walk
`latex_within_boundary` answered a question about path COMPONENTS with
string arithmetic:
dir:sub(1, #boundary + 1) == boundary .. "/"
With a `/` boundary the needle is `"//"`, which no canonical path
begins with. Every ancestor was therefore judged out of bounds, the
marker walk never examined a single directory, and each chapter of a
thesis got its own root — two texlab processes for one document tree.
The lane's headline behaviour, silently off, with all fourteen shipped
tests still green because every one of them clamps the boundary to its
own tempdir.
Fixed by comparing segments rather than characters, so the root is a
boundary with zero segments — containing everything by construction
instead of by a special case, and tolerating a trailing separator for
free. The same root-is-special trap sat at two other points on the same
path and is closed with it:
* `latex_parent_of` returned nil for a top-level directory, making `/`
the one directory the walk could never examine — the identical bug
from the far end. It now yields `/`, matching `walk_for_marker`'s
`Path::ancestors` on the Rust side, and still terminates because `/`
has no component to strip.
* `latex_root_for` sliced `/paper.tex` to an EMPTY directory, which
canonicalizes to nothing and made the resolver DECLINE — and a
decline is the one path that falls through to `pmacs.project.detect`,
whose walk includes `.git`. A document at the filesystem root now
roots at `/`.
* `latex_marker_in`'s join is guarded for `dir == "/"`, which this
change makes reachable for the first time; the naive form produces
`//name`, the one spelling POSIX leaves implementation-defined.
Two new pins plus a strengthened one, 16 tests:
* `two_chapters_share_one_server_under_a_root_search_boundary` — the
defect end to end through ATTACH, not on the predicate, because the
symptom is two servers rather than a wrong string. Restoring the old
comparison fails exactly this test, with the two-server output.
* `latex_root_walk_stops_at_the_search_boundary` now asserts BOTH
directions. "Stops at the boundary" is also satisfied by a walk that
never runs — which is precisely what a `/` boundary produced — so the
hermeticity property (R8's shape: a stray `latexmkrc` above the
tempdir must stay invisible) is now paired with the walk still
climbing to, and examining, the boundary directory itself.
* `latex_root_for_a_document_at_the_filesystem_root_is_the_root`.
Also corrects `docs/active-work.md`: §3 no longer awaits a revision 3 —
`b5eaf27` IS revision 3 — and the lane entry now records that boundary
handling has been this resolver's interesting part twice, so a reader
weighing whether to trust it knows where to look first.
Gates: ALL GREEN via `./scripts/gate --acceptance lsp_latex_acceptance`.
`/tmp` on this machine is a near-full tmpfs and three grep tests fail
there with `QuotaExceeded`; the green run used
`TMPDIR=/home/jeans/build/pmacs-gate-tmp/lsp-latex`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
This commit is contained in:
parent
12b7ee58d8
commit
9a1e11c0c8
|
|
@ -337,8 +337,14 @@ local LATEX_ROOT_MARKERS = {
|
||||||
-- establishes it: content -> no error; empty file -> nil, no error;
|
-- establishes it: content -> no error; empty file -> nil, no error;
|
||||||
-- directory -> nil, "Is a directory"; missing -> `io.open` nil.
|
-- directory -> nil, "Is a directory"; missing -> `io.open` nil.
|
||||||
local function latex_marker_in(dir)
|
local function latex_marker_in(dir)
|
||||||
|
-- Joining, not testing: `/` is the one directory that already ends in
|
||||||
|
-- a separator, and `dir .. "/" .. name` would give `//name` — the
|
||||||
|
-- exactly-two-leading-slashes spelling POSIX leaves implementation-
|
||||||
|
-- defined. `/` became reachable here once the walk stopped treating
|
||||||
|
-- the filesystem root as off-limits, so the join has to say so.
|
||||||
|
local base = (dir == "/") and "" or dir
|
||||||
for _, name in ipairs(LATEX_ROOT_MARKERS) do
|
for _, name in ipairs(LATEX_ROOT_MARKERS) do
|
||||||
local f = io.open(dir .. "/" .. name, "r")
|
local f = io.open(base .. "/" .. name, "r")
|
||||||
if f then
|
if f then
|
||||||
local _, err = f:read(1)
|
local _, err = f:read(1)
|
||||||
f:close()
|
f:close()
|
||||||
|
|
@ -348,9 +354,18 @@ local function latex_marker_in(dir)
|
||||||
return false
|
return false
|
||||||
end
|
end
|
||||||
|
|
||||||
|
-- `/` is a directory like any other. The pattern below yields the EMPTY
|
||||||
|
-- string for a top-level directory (`/tmp` -> ``), and treating that as
|
||||||
|
-- "no parent" would make the filesystem root the one directory this walk
|
||||||
|
-- can never examine — the same root-is-special bug the boundary test
|
||||||
|
-- below had, from the other end. `/` itself matches nothing (no
|
||||||
|
-- non-separator component to strip), so the walk still terminates there.
|
||||||
|
-- This matches `walk_for_marker`'s use of `Path::ancestors`
|
||||||
|
-- (`src/project.rs`), which likewise ends at `/` inclusive.
|
||||||
local function latex_parent_of(dir)
|
local function latex_parent_of(dir)
|
||||||
local up = dir:match("^(.*)/[^/]+$")
|
local up = dir:match("^(.*)/[^/]+$")
|
||||||
if up == nil or up == dir or up == "" then return nil end
|
if up == nil or up == dir then return nil end
|
||||||
|
if up == "" then return "/" end
|
||||||
return up
|
return up
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
@ -360,9 +375,37 @@ end
|
||||||
-- ignored the boundary would break that contract — and make this
|
-- ignored the boundary would break that contract — and make this
|
||||||
-- resolver's own acceptance fixtures non-hermetic against any
|
-- resolver's own acceptance fixtures non-hermetic against any
|
||||||
-- `latexmkrc` sitting above the test's tempdir (R8's shape exactly).
|
-- `latexmkrc` sitting above the test's tempdir (R8's shape exactly).
|
||||||
|
--
|
||||||
|
-- Containment is a question about PATH COMPONENTS, so it is answered by
|
||||||
|
-- comparing components. The previous string-prefix form
|
||||||
|
-- (`dir:sub(1, #boundary + 1) == boundary .. "/"`) silently disabled the
|
||||||
|
-- entire walk for a `/` boundary: the needle became `"//"`, which no
|
||||||
|
-- canonical path begins with, so every ancestor was judged out of
|
||||||
|
-- bounds, no marker was ever examined, and each chapter of a thesis got
|
||||||
|
-- its own server. Segment comparison makes the root boundary a boundary
|
||||||
|
-- with zero segments — containing everything, by construction rather
|
||||||
|
-- than by a special case — and absorbs a trailing separator for free.
|
||||||
|
--
|
||||||
|
-- Both arguments are canonical absolute paths (`latex_root_for`
|
||||||
|
-- canonicalizes `dir`; `set_search_boundary` canonicalizes the boundary
|
||||||
|
-- at set time), so a leading-separator mismatch cannot arise.
|
||||||
|
local function latex_path_segments(path)
|
||||||
|
local segs = {}
|
||||||
|
for seg in path:gmatch("[^/]+") do
|
||||||
|
segs[#segs + 1] = seg
|
||||||
|
end
|
||||||
|
return segs
|
||||||
|
end
|
||||||
|
|
||||||
local function latex_within_boundary(dir, boundary)
|
local function latex_within_boundary(dir, boundary)
|
||||||
if not boundary then return true end
|
if not boundary then return true end
|
||||||
return dir == boundary or dir:sub(1, #boundary + 1) == boundary .. "/"
|
local want = latex_path_segments(boundary)
|
||||||
|
local have = latex_path_segments(dir)
|
||||||
|
if #have < #want then return false end
|
||||||
|
for i = 1, #want do
|
||||||
|
if have[i] ~= want[i] then return false end
|
||||||
|
end
|
||||||
|
return true
|
||||||
end
|
end
|
||||||
|
|
||||||
-- Returns the INNERMOST ancestor holding a texlab root marker, or the
|
-- Returns the INNERMOST ancestor holding a texlab root marker, or the
|
||||||
|
|
@ -382,6 +425,11 @@ local function latex_root_for(path)
|
||||||
if type(path) ~= "string" then return nil end
|
if type(path) ~= "string" then return nil end
|
||||||
local dir = path:match("^(.*)/[^/]*$")
|
local dir = path:match("^(.*)/[^/]*$")
|
||||||
if not dir then return nil end
|
if not dir then return nil end
|
||||||
|
-- Same root-is-special trap as `latex_parent_of`: `/paper.tex` slices
|
||||||
|
-- to an EMPTY directory, which canonicalizes to nothing and would make
|
||||||
|
-- the resolver DECLINE — and a decline is the one path that reaches
|
||||||
|
-- `pmacs.project.detect`, whose walk includes `.git`.
|
||||||
|
if dir == "" then dir = "/" end
|
||||||
dir = pmacs.fs.canonicalize(dir)
|
dir = pmacs.fs.canonicalize(dir)
|
||||||
if not dir then return nil end
|
if not dir then return nil end
|
||||||
local boundary
|
local boundary
|
||||||
|
|
|
||||||
|
|
@ -333,11 +333,12 @@ authoritative tip** — the ref, not a SHA. Recover with
|
||||||
- **`args = {}` is also observed**, not assumed: bare `texlab`
|
- **`args = {}` is also observed**, not assumed: bare `texlab`
|
||||||
answers `initialize` with `TexLab 5.25.1` over stdio, so the `run`
|
answers `initialize` with `TexLab 5.25.1` over stdio, so the `run`
|
||||||
subcommand is not needed.
|
subcommand is not needed.
|
||||||
- **§3 therefore reads slightly stale** — it frames marker 1 as
|
- **§3 said the wrong thing and has been corrected — `b5eaf27` IS
|
||||||
conditional on texlab honouring the file, when the operative fact
|
revision 3.** It framed marker 1 as conditional on texlab honouring
|
||||||
is that texlab honours the *client-supplied root*. Worth a revision
|
the `.texlabroot` *file*, when the operative fact is that texlab
|
||||||
3 by whoever next touches the document; not smuggled into this
|
honours the *client-supplied root* and never widens it. The caveat
|
||||||
lane's commit.
|
was discharged by observation, and revision 3 records what that
|
||||||
|
established. Nothing about §3 is outstanding.
|
||||||
- **`.git` exclusion needed more than omitting it from the list.**
|
- **`.git` exclusion needed more than omitting it from the list.**
|
||||||
`project_root_for` falls through to `pmacs.project.detect` when a
|
`project_root_for` falls through to `pmacs.project.detect` when a
|
||||||
resolver returns nil, and **that** walk includes `.git` — so a
|
resolver returns nil, and **that** walk includes `.git` — so a
|
||||||
|
|
@ -360,6 +361,31 @@ authoritative tip** — the ref, not a SHA. Recover with
|
||||||
(1), boundary ignored (1), `io.open` truthiness so a directory counts
|
(1), boundary ignored (1), `io.open` truthiness so a directory counts
|
||||||
as a marker (1), marker set narrowed (4), command renamed with
|
as a marker (1), marker set narrowed (4), command renamed with
|
||||||
opinionated settings added (1).
|
opinionated settings added (1).
|
||||||
|
- **The boundary has now been the interesting part twice, and the
|
||||||
|
second time it was a real defect (fixed in review).** First it was
|
||||||
|
hermeticity — every fixture sets `set_search_boundary` at its own
|
||||||
|
tempdir because R8's shape (a stray `latexmkrc` above the tempdir)
|
||||||
|
would make the markerless assertions pass while testing nothing.
|
||||||
|
Then review found `latex_within_boundary` answering a PATH question
|
||||||
|
with string arithmetic: `dir:sub(1, #boundary + 1) == boundary .. "/"`
|
||||||
|
compares against `"//"` when the boundary is `/`, which no canonical
|
||||||
|
path matches, so a root boundary judged **every** ancestor out of
|
||||||
|
bounds, ran no marker walk at all, and gave each chapter of a thesis
|
||||||
|
its own server — the lane's headline behaviour silently off, with
|
||||||
|
every shipped test still green because each one clamps to a tempdir.
|
||||||
|
The same trap sat at the other end (`/` was never a walk candidate,
|
||||||
|
and `/paper.tex` sliced to an empty directory and declined into the
|
||||||
|
`.git`-aware detector). Now segment comparison throughout: the root
|
||||||
|
is a boundary with zero segments, contained by construction rather
|
||||||
|
than by a special case. Pinned by an ATTACH-level test under a `/`
|
||||||
|
boundary — two chapters, one server, marker root — and the
|
||||||
|
hermeticity property asserts **both** directions, since "stops at the
|
||||||
|
boundary" is also satisfied by a walk that never runs. Suite is 16
|
||||||
|
tests. **A reader
|
||||||
|
deciding whether to trust this resolver should read it as: the marker
|
||||||
|
set and the `.git` exclusion were settled by observation and are
|
||||||
|
solid; the boundary arithmetic around them was not, and is the place
|
||||||
|
to look first if roots come back wrong.**
|
||||||
- **Trap for the next agent in this worktree:** this machine exports a
|
- **Trap for the next agent in this worktree:** this machine exports a
|
||||||
shared `CARGO_TARGET_DIR`, so a bare `cargo test` compiles against a
|
shared `CARGO_TARGET_DIR`, so a bare `cargo test` compiles against a
|
||||||
sibling worktree's artifacts and fails with errors from code that is
|
sibling worktree's artifacts and fails with errors from code that is
|
||||||
|
|
|
||||||
|
|
@ -488,6 +488,54 @@ fn two_chapters_of_one_thesis_share_a_single_server() {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn two_chapters_share_one_server_under_a_root_search_boundary() {
|
||||||
|
// A `/` boundary is "clamp nothing", spelled as a path — and it used
|
||||||
|
// to disable the marker walk OUTRIGHT. The containment test was
|
||||||
|
// string arithmetic (`dir:sub(1, #boundary + 1) == boundary .. "/"`),
|
||||||
|
// so a `/` boundary asked whether each ancestor began with `"//"`,
|
||||||
|
// which no canonical path does. Every ancestor was judged out of
|
||||||
|
// bounds, no marker was ever examined, and each chapter got its own
|
||||||
|
// root — the lane's headline behaviour, silently off, with the
|
||||||
|
// predicate's unit-level answers all still looking plausible.
|
||||||
|
//
|
||||||
|
// Pinned through ATTACH because that is where the symptom lives: two
|
||||||
|
// texlab processes for one thesis, not a wrong string.
|
||||||
|
//
|
||||||
|
// Still hermetic despite the unclamped boundary: innermost marker
|
||||||
|
// wins, and `thesis/` has one, so no `latexmkrc` above the tempdir
|
||||||
|
// can change the answer.
|
||||||
|
let fx = Fixture::new();
|
||||||
|
let mut state = editor();
|
||||||
|
exec(&state, "pmacs.project.set_search_boundary(\"/\")");
|
||||||
|
let seen: String = eval(&state, "return pmacs.project.search_boundary() or \"\"");
|
||||||
|
assert_eq!(
|
||||||
|
seen, "/",
|
||||||
|
"fixture precondition: the boundary must be the filesystem root"
|
||||||
|
);
|
||||||
|
point_command_at(&state, &fake_lsp_path());
|
||||||
|
fx.write("thesis/latexmkrc", "");
|
||||||
|
let one = fx.write("thesis/chapters/one.tex", "\\section{One}\n");
|
||||||
|
let two = fx.write("thesis/appendix/two.tex", "\\section{Two}\n");
|
||||||
|
open(&state, &one);
|
||||||
|
settle(&mut state);
|
||||||
|
open(&state, &two);
|
||||||
|
settle(&mut state);
|
||||||
|
|
||||||
|
let rows = rows(&state);
|
||||||
|
assert_eq!(
|
||||||
|
rows.len(),
|
||||||
|
1,
|
||||||
|
"a root boundary must behave like any other boundary: both \
|
||||||
|
chapters resolve to the thesis root, so ONE server: {rows:?}"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
rows[0].split('|').nth(1).unwrap(),
|
||||||
|
file_uri(&fx.dir("thesis")),
|
||||||
|
"and that one server is rooted at the marker directory"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn two_markerless_documents_in_different_directories_do_not_share_a_server() {
|
fn two_markerless_documents_in_different_directories_do_not_share_a_server() {
|
||||||
// The complement of the pin above: the fallback is the file's own
|
// The complement of the pin above: the fallback is the file's own
|
||||||
|
|
@ -604,6 +652,47 @@ fn latex_root_walk_stops_at_the_search_boundary() {
|
||||||
"the walk must not climb past the search boundary to reach the \
|
"the walk must not climb past the search boundary to reach the \
|
||||||
marker above it"
|
marker above it"
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// The other direction, and it is not decoration: "stops at the
|
||||||
|
// boundary" is also satisfied by a walk that never runs at all —
|
||||||
|
// which is precisely what a `/` boundary used to produce. So assert
|
||||||
|
// that within the boundary the walk still CLIMBS, and that the
|
||||||
|
// boundary directory itself is a candidate (inclusive, matching
|
||||||
|
// `set_search_boundary`'s documented contract).
|
||||||
|
fx.write("inner/.texlabroot", "");
|
||||||
|
assert_eq!(
|
||||||
|
resolve_root(&state, &doc).as_deref(),
|
||||||
|
Some(inner.display().to_string().as_str()),
|
||||||
|
"a marker AT the boundary directory is found, and the walk \
|
||||||
|
climbs out of `chapters/` to reach it"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn latex_root_for_a_document_at_the_filesystem_root_is_the_root() {
|
||||||
|
// The same root-is-special trap one level up: `/paper.tex` slices to
|
||||||
|
// an EMPTY directory string, which canonicalizes to nothing, so the
|
||||||
|
// resolver DECLINED — and a decline is the one path that falls
|
||||||
|
// through to `pmacs.project.detect`, whose walk includes `.git`.
|
||||||
|
// Hermetic: the boundary is this fixture's tempdir, so `/` is out of
|
||||||
|
// bounds, no marker is examined, and the answer is the directory
|
||||||
|
// itself regardless of what sits at the filesystem root.
|
||||||
|
let fx = Fixture::new();
|
||||||
|
let state = editor();
|
||||||
|
fx.bind(&state);
|
||||||
|
let doc = Path::new("/pmacs-lsp-latex-no-such-document.tex");
|
||||||
|
assert!(
|
||||||
|
!doc.exists(),
|
||||||
|
"fixture precondition: {} must not exist",
|
||||||
|
doc.display()
|
||||||
|
);
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
resolve_root(&state, doc).as_deref(),
|
||||||
|
Some("/"),
|
||||||
|
"a document at the filesystem root roots at `/`; it must not \
|
||||||
|
decline into the shared `.git`-aware detector"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[path = "common/iso.rs"]
|
#[path = "common/iso.rs"]
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue