Skip to content

Commit 71c33e6

Browse files
fix(nvim): Correctly handle different cwds (#410)
* fix(nvim): Correctly handle different cwds * fix(ci): suppress missing-fields diagnostic in test file * fix(test): clear DirChanged autocmd to prevent race in e2e suite * fix(test): normalize paths for cross-platform comparison in e2e * fix(test): robust path normalization for Windows 8.3 names and case * fix(test): use fs_realpath for Windows 8.3 name expansion * fix(test): constrain grep test to .rs files to avoid flaky ordering * fix(constraints): double-negation bug in Not(Glob(...)) filtering
1 parent acd2f0c commit 71c33e6

5 files changed

Lines changed: 248 additions & 33 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ test-rust:
2121

2222
test-lua: test-setup build
2323
nvim --headless -u tests/minimal_init.lua \
24-
-c "PlenaryBustedFile tests/fff_core_spec.lua" 2>&1
24+
-c "PlenaryBustedDirectory tests/ {minimal_init = 'tests/minimal_init.lua'}" 2>&1
2525

2626
test-version: test-setup
2727
nvim --headless -u tests/minimal_init.lua \

crates/fff-core/src/constraints.rs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,15 +362,19 @@ fn collect_glob_indices<'a>(
362362
constraint: &Constraint<'a>,
363363
paths: &[&str],
364364
results: &mut Vec<(bool, AHashSet<usize>)>,
365-
is_negated: bool,
365+
_is_negated: bool,
366366
) {
367367
match constraint {
368368
Constraint::Glob(pattern) => {
369369
let indices = match_glob_pattern(pattern, paths);
370-
results.push((is_negated, indices));
370+
// Negation is handled by the `negate` parameter in
371+
// `item_matches_constraint_at_index`, NOT here. Storing
372+
// `is_negated=true` caused a double-negation bug when the
373+
// Glob arm also applied `negate`.
374+
results.push((false, indices));
371375
}
372376
Constraint::Not(inner) => {
373-
collect_glob_indices(inner, paths, results, !is_negated);
377+
collect_glob_indices(inner, paths, results, true);
374378
}
375379
_ => {}
376380
}
@@ -676,4 +680,38 @@ mod tests {
676680
));
677681
assert!(path_ends_with_suffix(path2, "세부_커리큘럼_최종.csv"));
678682
}
683+
684+
#[test]
685+
fn test_negated_glob_excludes_matching_files() {
686+
let arena_ptr = ArenaPtr(std::ptr::null());
687+
688+
let items = vec![
689+
TestItem {
690+
relative_path: "src/main.rs",
691+
file_name: "main.rs",
692+
},
693+
TestItem {
694+
relative_path: "src/lib.ts",
695+
file_name: "lib.ts",
696+
},
697+
TestItem {
698+
relative_path: "include/fff.h",
699+
file_name: "fff.h",
700+
},
701+
];
702+
703+
// Not(Glob("**/*.rs")) should exclude .rs files
704+
let constraints = vec![Constraint::Not(Box::new(Constraint::Glob("**/*.rs")))];
705+
let result = apply_constraints(&items, &constraints, arena_ptr).unwrap();
706+
let paths: Vec<&str> = result.iter().map(|i| i.relative_path).collect();
707+
assert!(
708+
!paths.contains(&"src/main.rs"),
709+
"rs file should be excluded"
710+
);
711+
assert!(paths.contains(&"src/lib.ts"), "ts file should be included");
712+
assert!(
713+
paths.contains(&"include/fff.h"),
714+
"h file should be included"
715+
);
716+
}
679717
}

lua/fff/picker_ui.lua

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,26 @@ local list_renderer = require('fff.list_renderer')
1010
local scrollbar = require('fff.scrollbar')
1111
local rust = require('fff.rust')
1212

13+
--- Base path of picker can change that's why we can not rely on relative
14+
--- path for reading/opening files. This function resolves correct absolute path
15+
--- @param relative_path string|nil
16+
--- @return string|nil
17+
local function canonicalize_fff_path(relative_path)
18+
if not relative_path or relative_path == '' then return nil end
19+
local path = relative_path
20+
-- Strip Windows long-path prefix (\\?\) — Neovim cannot open these.
21+
if vim.startswith(path, '\\\\?\\') then path = path:sub(5) end
22+
-- Already absolute: don't re-anchor.
23+
if vim.fn.fnamemodify(path, ':p') == path then return path end
24+
local base = conf.get().base_path
25+
if not base or base == '' then return path end
26+
return vim.fs.normalize(base .. '/' .. path)
27+
end
28+
29+
--- @param item table|nil
30+
--- @return string|nil
31+
local function resolve_item_path(item) return item and canonicalize_fff_path(item.relative_path) or nil end
32+
1333
local BORDER_PRESETS = {
1434
single = { '', '', '', '', '', '', '', '' },
1535
double = { '', '', '', '', '', '', '', '' },
@@ -1869,7 +1889,7 @@ function M.update_preview()
18691889
if M.state.file_info_buf then preview.update_file_info_buffer(item, M.state.file_info_buf, M.state.cursor) end
18701890

18711891
preview.set_preview_window(M.state.preview_win)
1872-
preview.preview(item.relative_path, M.state.preview_buf, effective_location, item.is_binary)
1892+
preview.preview(resolve_item_path(item), M.state.preview_buf, effective_location, item.is_binary)
18731893
end
18741894

18751895
--- Clear preview
@@ -2314,12 +2334,15 @@ function M.send_to_quickfix()
23142334
if has_selections then
23152335
-- Use explicitly selected items (survives page changes)
23162336
for _, item in pairs(M.state.selected_items) do
2317-
table.insert(qf_list, {
2318-
filename = item.relative_path,
2319-
lnum = item.line_number or 1,
2320-
col = (item.col or 0) + 1,
2321-
text = item.line_content or vim.fn.fnamemodify(item.relative_path, ':.'),
2322-
})
2337+
local abs = resolve_item_path(item)
2338+
if abs then
2339+
table.insert(qf_list, {
2340+
filename = abs,
2341+
lnum = item.line_number or 1,
2342+
col = (item.col or 0) + 1,
2343+
text = item.line_content or vim.fn.fnamemodify(abs, ':.'),
2344+
})
2345+
end
23232346
end
23242347
else
23252348
-- No selections: run an exhaustive search to get all matches
@@ -2334,12 +2357,13 @@ function M.send_to_quickfix()
23342357
end
23352358

23362359
for _, item in ipairs(all_items) do
2337-
if item and item.relative_path then
2360+
local abs = resolve_item_path(item)
2361+
if abs then
23382362
table.insert(qf_list, {
2339-
filename = item.relative_path,
2363+
filename = abs,
23402364
lnum = item.line_number or 1,
23412365
col = (item.col or 0) + 1,
2342-
text = item.line_content or vim.fn.fnamemodify(item.relative_path, ':.'),
2366+
text = item.line_content or vim.fn.fnamemodify(abs, ':.'),
23432367
})
23442368
end
23452369
end
@@ -2348,14 +2372,16 @@ function M.send_to_quickfix()
23482372
-- Normal file mode: per-file entries at line 1
23492373
local paths = {}
23502374

2351-
-- Collect from explicit selections, or fall back to all visible items
2375+
-- Collect from explicit selections, or fall back to all visible items.
2376+
-- selected_files is keyed by relative_path; filtered_items carries relative_path too.
23522377
if next(M.state.selected_files) then
2353-
for path, _ in pairs(M.state.selected_files) do
2354-
table.insert(paths, path)
2378+
for relative_path, _ in pairs(M.state.selected_files) do
2379+
table.insert(paths, canonicalize_fff_path(relative_path))
23552380
end
23562381
else
23572382
for _, item in ipairs(M.state.filtered_items) do
2358-
if item and item.relative_path then table.insert(paths, item.relative_path) end
2383+
local abs = resolve_item_path(item)
2384+
if abs then table.insert(paths, abs) end
23592385
end
23602386
end
23612387

@@ -2398,14 +2424,12 @@ function M.select(action)
23982424

23992425
action = action or 'edit'
24002426

2401-
-- Strip Windows long path prefix (\\?\) if present.
2402-
-- These can surface from Rust's fs::canonicalize on Windows when LongPathsEnabled is set.
2403-
-- Neovim cannot open paths with this prefix. The Rust side uses dunce::canonicalize to avoid
2404-
-- producing these, but we strip defensively here as well.
2405-
local path = item.relative_path
2406-
if vim.startswith(path, '\\\\?\\') then path = path:sub(5) end
2407-
2408-
local relative_path = vim.fn.fnamemodify(path, ':.')
2427+
-- Anchor against the indexer's base_path (may differ from cwd), then rephrase
2428+
-- as cwd-relative for a nicer buffer name when possible. When outside cwd,
2429+
-- fnamemodify(':.') leaves the absolute path intact.
2430+
local abs_path = resolve_item_path(item)
2431+
if not abs_path then return end
2432+
local relative_path = vim.fn.fnamemodify(abs_path, ':.')
24092433
local location = M.state.location -- Capture location before closing
24102434
local query = M.state.query -- Capture query before closing for tracking
24112435
local mode = M.state.mode -- Capture mode before closing for tracking

packages/fff-node/test/e2e.mjs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,17 @@ describe("fff-node", { concurrency: 1 }, () => {
132132

133133
describe("grep", { concurrency: 1 }, () => {
134134
it("finds FffResult in Rust sources", () => {
135-
const r = finder.grep("FffResult", { mode: "plain" });
136-
assert.ok(r.ok, `grep failed: ${!r.ok ? r.error : ""}`);
137-
assert.ok(r.value.items.length > 0);
138-
assert.ok(r.value.items.some((m) => m.relativePath.endsWith(".rs")));
135+
// Constrain to .rs files so the assertion doesn't depend on result ordering
136+
// or content-indexing timing for other file types.
137+
const rustResults = finder.grep("*.rs FffResult", { mode: "plain" });
138+
assert.ok(rustResults.ok, `grep failed: ${!rustResults.ok ? rustResults.error : ""}`);
139+
assert.ok(rustResults.value.items.length > 0, "expected at least one .rs match");
140+
assert.ok(rustResults.value.items.some((m) => m.relativePath.endsWith(".rs")));
141+
142+
const cResults = finder.grep("!**/*.{js,ts,rs} FffResult", { mode: "plain" });
143+
assert.ok(cResults.ok, `grep failed: ${!cResults.ok ? cResults.error : ""}`);
144+
assert.ok(cResults.value.items.length > 0, "expected at least one non-js/ts/rs match");
145+
assert.ok(cResults.value.items.some((m) => m.relativePath.endsWith(".h")));
139146
});
140147

141148
it("match items contain all required fields", () => {
@@ -190,7 +197,9 @@ describe("fff-node", { concurrency: 1 }, () => {
190197
.map((m) => normalizePath(m.relativePath))
191198
.join(", ")}`,
192199
);
193-
assert.deepEqual(match.contextBefore, [" if (raw.context_before_count > 0) {"]);
200+
assert.deepEqual(match.contextBefore, [
201+
" if (raw.context_before_count > 0) {",
202+
]);
194203
assert.deepEqual(match.contextAfter, [" }"]);
195204
});
196205
});
@@ -217,12 +226,10 @@ describe("fff-node", { concurrency: 1 }, () => {
217226
assert.ok(r.value > 0);
218227
});
219228

220-
221229
it("isScanning returns a boolean", () => {
222230
assert.equal(typeof finder.isScanning(), "boolean");
223231
});
224232

225-
226233
describe("healthCheck", { concurrency: 1 }, () => {
227234
it("reports initialized state with instance", () => {
228235
const r = finder.healthCheck(REPO_ROOT);
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
---@diagnostic disable: undefined-field, missing-fields
2+
-- Regression test for https://github.com/dmtrKovalenko/fff.nvim/issues/389
3+
-- When find_files_in_dir(dir) runs with dir != neovim's cwd, the Rust indexer
4+
-- reports paths relative to `dir`, but the Lua side used to resolve them
5+
-- against neovim's cwd when calling :edit / preview / quickfix — so files
6+
-- opened as phantom buffers and previews showed "No preview available".
7+
8+
local fff_rust = require('fff.rust')
9+
local picker_ui = require('fff.picker_ui')
10+
local file_picker = require('fff.file_picker')
11+
12+
--- Normalise a path so that comparisons work on every OS.
13+
--- Windows complicates things: Rust may return forward-slash paths while
14+
--- vim.fn.resolve uses backslashes, temp paths may contain 8.3 short names
15+
--- (RUNNER~1), and the filesystem is case-insensitive.
16+
--- vim.uv.fs_realpath expands 8.3 names on Windows (unlike vim.fn.resolve).
17+
--- @param p string
18+
--- @return string
19+
local function norm(p)
20+
-- fs_realpath is the closest Lua equivalent of Rust's std::fs::canonicalize
21+
-- and expands 8.3 short names on Windows.
22+
local rp = vim.uv.fs_realpath(p) or vim.fn.fnamemodify(vim.fn.resolve(p), ':p')
23+
local n = vim.fs.normalize(rp)
24+
-- Strip trailing slash for consistent comparison
25+
n = n:gsub('/$', '')
26+
-- Case-fold on Windows (drive letters, 8.3 short names, etc.)
27+
if vim.fn.has('win32') == 1 then n = n:lower() end
28+
return n
29+
end
30+
31+
--- `change_indexing_directory` swaps the picker on a background thread, so the
32+
--- `FILE_PICKER` global may still point at the *old* picker for a moment —
33+
--- `wait_for_initial_scan` on the old picker returns immediately and the
34+
--- search then runs against the new, still-empty index. Poll `health_check`
35+
--- until `base_path` matches the expected dir before waiting on the scan.
36+
local function wait_for_reindex(expected_dir, timeout_ms)
37+
local expected = norm(expected_dir)
38+
local deadline = vim.uv.hrtime() + timeout_ms * 1e6
39+
while vim.uv.hrtime() < deadline do
40+
local ok, health = pcall(fff_rust.health_check, expected)
41+
if ok and health and health.file_picker and health.file_picker.base_path then
42+
if norm(health.file_picker.base_path) == expected then return true end
43+
end
44+
vim.wait(20, function() return false end)
45+
end
46+
return false
47+
end
48+
49+
local function wait_for_scan(expected_dir, timeout_ms)
50+
assert.is_true(wait_for_reindex(expected_dir, timeout_ms), 'reindex to ' .. expected_dir .. ' did not complete')
51+
fff_rust.wait_for_initial_scan(timeout_ms)
52+
end
53+
54+
describe('picker find_files_in_dir path resolution (issue #389)', function()
55+
local sandbox_root, target_dir, other_cwd, target_filename
56+
57+
before_each(function()
58+
sandbox_root = vim.fn.tempname()
59+
target_dir = sandbox_root .. '/target-dir'
60+
other_cwd = sandbox_root .. '/other-cwd'
61+
vim.fn.mkdir(target_dir, 'p')
62+
vim.fn.mkdir(other_cwd, 'p')
63+
64+
target_filename = 'issue389_target.lua'
65+
local fd = assert(io.open(target_dir .. '/' .. target_filename, 'w'))
66+
fd:write('-- issue #389 regression fixture\nreturn true\n')
67+
fd:close()
68+
69+
-- Clear the DirChanged autocmd that a previous test run (e.g. fff_core_spec)
70+
-- may have installed. Without this, the :cd below triggers a scheduled
71+
-- change_indexing_directory(other_cwd) that races with our explicit
72+
-- change_indexing_directory(target_dir) and overwrites the FILE_PICKER.
73+
pcall(vim.api.nvim_del_augroup_by_name, 'fff_file_tracking')
74+
75+
vim.cmd('cd ' .. vim.fn.fnameescape(other_cwd))
76+
-- Equivalent to require('fff').setup({}) — just seeds vim.g.fff — but
77+
-- avoids the top-level fff module lookup which plenary's sandboxed
78+
-- require can miss depending on package.path.
79+
vim.g.fff = {}
80+
file_picker.setup()
81+
end)
82+
83+
after_each(function()
84+
pcall(picker_ui.close)
85+
pcall(fff_rust.stop_background_monitor)
86+
pcall(fff_rust.cleanup_file_picker)
87+
if sandbox_root then vim.fn.delete(sandbox_root, 'rf') end
88+
end)
89+
90+
it(':edit opens the file inside base_path even when neovim cwd differs', function()
91+
assert.are_not.equal(norm(target_dir), norm(vim.fn.getcwd()))
92+
93+
assert.is_true(picker_ui.change_indexing_directory(target_dir))
94+
wait_for_scan(target_dir, 10000)
95+
96+
local items = file_picker.search_files('', nil, nil, nil, nil)
97+
assert.is_true(#items > 0, 'indexer returned no items for target_dir (norm=' .. norm(target_dir) .. ')')
98+
99+
local target_item
100+
for _, item in ipairs(items) do
101+
if item.name == target_filename then
102+
target_item = item
103+
break
104+
end
105+
end
106+
assert.is_not_nil(target_item, 'target fixture file missing from results')
107+
-- On Windows Rust may use backslash separators; compare just the filename.
108+
local rel = target_item.relative_path
109+
assert.are.equal(target_filename, rel:match('[^/\\]+$') or rel)
110+
assert.is_nil(
111+
vim.uv.fs_stat(target_item.relative_path),
112+
'relative_path should not resolve against cwd — if it does, the test fixture is wrong'
113+
)
114+
115+
-- Drive the same code path that user hits when pressing <CR>: populate
116+
-- UI state as open_ui_with_state would and invoke select('edit').
117+
picker_ui.state.active = true
118+
picker_ui.state.filtered_items = items
119+
picker_ui.state.cursor = (function()
120+
for i, item in ipairs(items) do
121+
if item.name == target_filename then return i end
122+
end
123+
return 1
124+
end)()
125+
picker_ui.state.query = ''
126+
picker_ui.state.mode = nil
127+
picker_ui.state.location = nil
128+
picker_ui.state.suggestion_source = nil
129+
picker_ui.state.selected_files = {}
130+
picker_ui.state.selected_items = {}
131+
132+
picker_ui.select('edit')
133+
134+
local bufname = vim.api.nvim_buf_get_name(0)
135+
assert.is_true(bufname ~= '', 'expected :edit to open a buffer with a non-empty name')
136+
137+
local stat = vim.uv.fs_stat(bufname)
138+
assert.is_not_nil(stat, 'opened buffer points at a non-existent file: ' .. bufname)
139+
140+
-- The opened file must be the fixture inside target_dir, not a phantom
141+
-- file under cwd.
142+
local expected = norm(target_dir .. '/' .. target_filename)
143+
local actual = norm(bufname)
144+
assert.are.equal(expected, actual)
145+
end)
146+
end)

0 commit comments

Comments
 (0)