diff --git a/.agents/skills/verify-diff-lifecycle/covered.lock b/.agents/skills/verify-diff-lifecycle/covered.lock index 443ac6f98..1824dfe47 100644 --- a/.agents/skills/verify-diff-lifecycle/covered.lock +++ b/.agents/skills/verify-diff-lifecycle/covered.lock @@ -1,7 +1,7 @@ == lua/claudecode/config.lua file lua/claudecode/config.lua 3afb1bdeff27bfb6 == lua/claudecode/cwd.lua -file lua/claudecode/cwd.lua 35b7af3572d13018 +file lua/claudecode/cwd.lua b034e82c76e8dee4 == lua/claudecode/diff.lua file lua/claudecode/diff.lua ac01bc69a313e411 block resolve_saved 4d34b0c7e34d561f diff --git a/lua/claudecode/README.md b/lua/claudecode/README.md index fa33d527c..56cc549f8 100644 --- a/lua/claudecode/README.md +++ b/lua/claudecode/README.md @@ -128,6 +128,7 @@ Module map (direct files): - A per-call `opts_override` changes only a key that has a validator (`split_side`, `split_width_percentage`, `snacks_win_opts`, `cwd`, `git_repo_cwd`, `cwd_provider`) and a non-nil current value. So a per-call `cwd` or `cwd_provider` is ignored unless `setup` set one. Providers get only `split_side`, `split_width_percentage`, `auto_close`, `auto_insert`, `snacks_win_opts`, and the resolved `cwd` [terminal.lua:build_config] - The Claude command is `terminal_cmd`, or `claude` when it is unset. `no_proxy` and `NO_PROXY` always include `localhost`, `127.0.0.1`, and `::1` [terminal.lua:get_claude_command_and_env] - Terminal cwd precedence: `cwd_provider`, then static `cwd`, then the git root when `git_repo_cwd` is set [terminal.lua:build_config] +- `cwd.git_root` and `cwd.resolve` take a directory path as written: a folder name with `[`, `{`, `*`, `?`, `$`, `\`, a space or a comma stays that folder. A string that is not a directory is expanded (`~`, `$VAR`), and a string that starts with `~` or `$` is always expanded first, even when Neovim's current folder holds a folder with that literal name. The terminal does not use `cwd.resolve`: `terminal.lua:build_config` passes a static `cwd` and a `cwd_provider` result through `vim.fn.expand` itself, so there a folder name with `$` or `\` still changes. When `git rev-parse` cannot run or finds no repository, `git_root` walks up from `start_dir` made absolute with `:p`, with its `..` parts simplified, to the nearest folder that holds a `.git` folder. A relative `start_dir` that begins with `~` is not made absolute, because Vim reads `~` as the home folder and `~name` as a user's home folder, so the walk starts from there [cwd.lua:M.git_root] [cwd.lua:normalize_dir] [terminal.lua:build_config] - `send_to_terminal` never starts a terminal: without a valid terminal buffer it returns `false`. With `focus = true` it calls `M.open()` after a successful send, which can show a hidden terminal. It refuses the `none` and `external` providers, because they have no buffer. It turns CRLF and CR into LF and wraps multi-line text in bracketed-paste markers. It returns `false` when the buffer has no job channel or `chansend` fails [terminal.lua:M.send_to_terminal] - `utils.parse_command` does no globbing or variable expansion, so model aliases such as `opus[1m]` stay intact [utils.lua:M.parse_command] diff --git a/lua/claudecode/cwd.lua b/lua/claudecode/cwd.lua index 7eaeef84b..7451631ef 100644 --- a/lua/claudecode/cwd.lua +++ b/lua/claudecode/cwd.lua @@ -3,6 +3,13 @@ local M = {} +---Tell whether a path names an existing directory, as written: no expansion. +---@param path string +---@return boolean +local function is_directory(path) + return vim.fn.isdirectory(path) == 1 +end + ---Normalize and validate a directory path ---@param dir string|nil ---@return string|nil @@ -10,13 +17,16 @@ local function normalize_dir(dir) if type(dir) ~= "string" or dir == "" then return nil end + -- A directory that exists is taken as written. `vim.fn.expand` reads `[`, `{`, `*`, `?`, `$` and `\` in a name as + -- patterns or variables, so it would change the path of a folder that has them, or find none. Text that starts with + -- `~` or `$` asks for expansion, as in a shell and in `:cd`, so it goes through `expand` first: `is_directory` does not + -- expand, and a stray folder named `~` in Neovim's current folder must not take the place of the home folder. + if not dir:match("^[~$]") and is_directory(dir) then + return dir + end -- Expand ~ and similar local expanded = vim.fn.expand(dir) - local isdir = 1 - if vim.fn.isdirectory then - isdir = vim.fn.isdirectory(expanded) - end - if isdir == 1 then + if is_directory(expanded) then return expanded end return nil @@ -51,16 +61,21 @@ function M.git_root(start_dir) end end - -- Fallback: search for .git directory upward - if vim.fn.finddir then - local git_dir = vim.fn.finddir(".git", start_dir .. ";") - if type(git_dir) == "string" and git_dir ~= "" then - local parent = vim.fn.fnamemodify(git_dir, ":h") - return normalize_dir(parent) + -- Fallback: search for .git directory upward. The walk tests each folder as written. `vim.fn.finddir` reads its path + -- as a list ('path' option syntax), so a space or a comma in the path split it, and a `;` ended it. It starts from the + -- full path, so a relative `start_dir` also reaches the folders above the current one. + -- Before Neovim 0.11, `:p` keeps a last `..`, and `:h` of `a/b/..` is `a/b`: simplify first. + local dir = vim.fn.fnamemodify(vim.fn.simplify(vim.fn.fnamemodify(start_dir, ":p")), ":h") + while true do + if vim.fn.isdirectory(dir .. "/.git") == 1 then + return dir end + local parent = vim.fn.fnamemodify(dir, ":h") + if parent == dir then + return nil + end + dir = parent end - - return nil end ---Resolve the effective working directory based on terminal config and context diff --git a/tests/integration/cwd_path_characters_spec.lua b/tests/integration/cwd_path_characters_spec.lua new file mode 100644 index 000000000..50885c5a0 --- /dev/null +++ b/tests/integration/cwd_path_characters_spec.lua @@ -0,0 +1,274 @@ +if vim == nil or vim.fn.has("nvim") ~= 1 then + describe("the git root of a folder whose name holds unusual characters", function() + pending("needs a real Neovim") + end) + return +end + +if vim.fn.has("win32") == 1 then + describe("the git root of a folder whose name holds unusual characters", function() + pending("these names are not legal on Windows") + end) + return +end + +-- cwd.git_root finds the repository of a folder, so a repository in a folder with a space, a comma or a quote in its +-- name must be found like any other. `git rev-parse` is the first try. When git cannot run (it is not installed, or +-- not on PATH), a walk up to the nearest `.git` folder answers. Both took the path as a pattern: the walk read it as a +-- list of folders ('path' option syntax), and every lookup passed it to `vim.fn.expand`, which reads `[`, `{`, `*`, `?`, +-- `$` and `\` as patterns or variables. +describe("the git root of a folder whose name holds unusual characters", function() + local cwd = require("claudecode.cwd") + local root, empty_bin, saved + + -- Each name is legal in a path on Linux and macOS. The folder is the repository. + local names = { + { name = "no unusual character", folder = "repo" }, + { name = "a space", folder = "with space" }, + { name = "a comma", folder = "comma,dir" }, + { name = "a semicolon", folder = "semi;colon" }, + { name = "a single quote", folder = "it's" }, + { name = "double quotes", folder = 'say "hi"' }, + { name = "brackets and braces", folder = "[x] {y}" }, + { name = "parentheses", folder = "(a) (b)" }, + { name = "non-ASCII letters", folder = "ünï-cødé-日本語" }, + { name = "a star and a question mark", folder = "star*mark?" }, + { name = "a percent sign and a hash", folder = "percent%d#hash" }, + { name = "a backslash", folder = "back\\slash" }, + { name = "a dollar sign", folder = "$HOME-like" }, + { name = "a leading tilde", folder = "~tilde" }, + } + + --- Whether a folder above the test folders belongs to a repository, which would answer for a folder that has none. + local function repository_above(dir) + while true do + if vim.fn.isdirectory(dir .. "/.git") == 1 then + return true + end + local parent = vim.fn.fnamemodify(dir, ":h") + if parent == dir then + return false + end + dir = parent + end + end + + local function realpath(path) + return vim.loop.fs_realpath(path) or path + end + + --- Makes `//` with a `sub/deeper` folder in it. + ---@param make_repo fun(repo: string) Makes the folder a repository + local function repo_in(mode, folder, make_repo) + local repo = root .. "/" .. mode .. "/" .. folder + vim.fn.mkdir(repo .. "/sub/deeper", "p") + make_repo(repo) + return repo + end + + before_each(function() + root = vim.fn.tempname() + empty_bin = root .. "/empty-bin" + vim.fn.mkdir(empty_bin, "p") + saved = { path = vim.env.PATH, shell = vim.o.shell } + end) + + after_each(function() + vim.env.PATH, vim.o.shell = saved.path, saved.shell + vim.fn.delete(root, "rf") + end) + + --- Runs `fn` with git off PATH, as on a host that has no git. The shell keeps its absolute path, so a command that + --- names git ends with "not found" and shell_error 127, and a list command raises E475, as they do there. + local function without_git(fn) + local shell = vim.fn.exepath("sh") + assert.is_true(shell ~= "", "the test needs sh") + vim.o.shell, vim.env.PATH = shell, empty_bin + local ok, result = pcall(fn) + vim.o.shell, vim.env.PATH = saved.shell, saved.path + assert(ok, result) + return result + end + + describe("when git cannot run", function() + it("really cannot run git here", function() + without_git(function() + assert.equals(0, vim.fn.executable("git")) + end) + end) + + for _, case in ipairs(names) do + it("finds a repository whose folder has " .. case.name, function() + local repo = repo_in("no-git", case.folder, function(path) + vim.fn.mkdir(path .. "/.git", "p") + end) + + local from_root = without_git(function() + return cwd.git_root(repo) + end) + local from_subfolder = without_git(function() + return cwd.git_root(repo .. "/sub/deeper") + end) + + assert.equals(realpath(repo), from_root and realpath(from_root), "from the repository folder") + assert.equals(realpath(repo), from_subfolder and realpath(from_subfolder), "from a folder inside it") + -- The answer is the folder itself, not another spelling of it. + assert.is_nil(from_root and from_root:find("/$"), "no trailing slash") + end) + end + + -- A folder above the test folders can hold a repository, for example a TMPDIR inside a checkout. + local make_non_repo_test = repository_above(vim.fn.fnamemodify(vim.fn.tempname(), ":h")) and pending or it + make_non_repo_test("finds no repository for a folder that has none, whatever its name", function() + for _, case in ipairs(names) do + local folder = root .. "/non-repo/" .. case.folder + vim.fn.mkdir(folder, "p") + + local found = without_git(function() + return cwd.git_root(folder) + end) + + assert.is_nil(found, "a non-repository folder with " .. case.name) + end + end) + end) + + describe("when git runs", function() + if vim.fn.executable("git") ~= 1 then + pending("needs git") + return + end + + for _, case in ipairs(names) do + it("finds a repository whose folder has " .. case.name, function() + local repo = repo_in("git", case.folder, function(path) + vim.fn.system({ "git", "init", "-q", path }) + assert.equals(0, vim.v.shell_error, "git init") + end) + + local from_root = cwd.git_root(repo) + local from_subfolder = cwd.git_root(repo .. "/sub/deeper") + + assert.equals(realpath(repo), from_root and realpath(from_root), "from the repository folder") + assert.equals(realpath(repo), from_subfolder and realpath(from_subfolder), "from a folder inside it") + -- The answer is the folder itself, not another spelling of it. + assert.is_nil(from_root and from_root:find("/$"), "no trailing slash") + end) + end + end) + + describe("a start folder given relative to the current folder", function() + it("finds the repository above the current folder, when git cannot run", function() + local repo = repo_in("relative", "my repo", function(path) + vim.fn.mkdir(path .. "/.git", "p") + end) + local saved_cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(repo .. "/sub")) + + local from_dot = without_git(function() + return cwd.git_root(".") + end) + local from_child = without_git(function() + return cwd.git_root("deeper") + end) + vim.cmd("cd " .. vim.fn.fnameescape(saved_cwd)) + + assert.equals(realpath(repo), from_dot and realpath(from_dot), "from .") + assert.equals(realpath(repo), from_child and realpath(from_child), "from a folder inside the current one") + end) + it("goes up from a start folder that ends in .., when git cannot run", function() + local outer = repo_in("dotdot", "outer", function(path) + vim.fn.mkdir(path .. "/.git", "p") + end) + vim.fn.mkdir(outer .. "/a/b/.git", "p") + local saved_cwd = vim.fn.getcwd() + vim.cmd("cd " .. vim.fn.fnameescape(outer .. "/a/b")) + + local from_parent = without_git(function() + return cwd.git_root("..") + end) + vim.cmd("cd " .. vim.fn.fnameescape(saved_cwd)) + + -- ".." is outer/a, which has no .git folder: its repository is outer, not outer/a/b below it. + assert.equals(realpath(outer), from_parent and realpath(from_parent)) + end) + end) + + -- The next two blocks test `cwd.resolve`, not the terminal. `terminal.lua:build_config` does not call it: it expands + -- a static `cwd` and a `cwd_provider` result with `vim.fn.expand` itself, so these cases say nothing about what the + -- terminal does with a folder name that holds `$` or a backslash. + describe("cwd.resolve with git_repo_cwd", function() + it("returns the repository of a file folder whose name has a space and a bracket, when git cannot run", function() + local repo = repo_in("resolve", "my [repo]", function(path) + vim.fn.mkdir(path .. "/.git", "p") + end) + + local resolved = without_git(function() + return cwd.resolve({ git_repo_cwd = true }, { file_dir = repo .. "/sub", cwd = repo }) + end) + + assert.equals(realpath(repo), resolved and realpath(resolved)) + end) + end) + + describe("cwd.resolve with a folder given as a setting", function() + it("still expands ~ and $VAR when the text is not a folder", function() + local repo = repo_in("setting", "proj", function(path) + vim.fn.mkdir(path .. "/.git", "p") + end) + local saved_home, saved_var = vim.env.HOME, vim.env.CLAUDECODE_TEST_ROOT + vim.env.HOME, vim.env.CLAUDECODE_TEST_ROOT = root .. "/setting", root .. "/setting" + + local tilde = cwd.resolve({ cwd = "~/proj" }, {}) + local variable = cwd.resolve({ cwd = "$CLAUDECODE_TEST_ROOT/proj" }, {}) + vim.env.HOME, vim.env.CLAUDECODE_TEST_ROOT = saved_home, saved_var + + assert.equals(realpath(repo), tilde and realpath(tilde)) + assert.equals(realpath(repo), variable and realpath(variable)) + end) + + --- Runs `fn` with Neovim's current folder set to `dir` and HOME set to `home`, then puts both back. + local function with_cwd_and_home(dir, home, fn) + local saved_cwd, saved_home = vim.fn.getcwd(), vim.env.HOME + vim.cmd("cd " .. vim.fn.fnameescape(dir)) + vim.env.HOME = home + local ok, result = pcall(fn) + vim.env.HOME = saved_home + vim.cmd("cd " .. vim.fn.fnameescape(saved_cwd)) + assert.is_true(ok, result) + return result + end + + it("expands a leading ~ even when Neovim's cwd holds a folder named ~", function() + vim.fn.mkdir(root .. "/stray/home/sub", "p") + vim.fn.mkdir(root .. "/stray/work/~/sub", "p") + + local resolved = with_cwd_and_home(root .. "/stray/work", root .. "/stray/home", function() + return cwd.resolve({ cwd = "~/sub" }, {}) + end) + + assert.equals(realpath(root .. "/stray/home/sub"), resolved and realpath(resolved)) + end) + + it("expands a leading $ even when Neovim's cwd holds a folder named $HOME", function() + vim.fn.mkdir(root .. "/stray/home", "p") + vim.fn.mkdir(root .. "/stray/work/$HOME", "p") + + local resolved = with_cwd_and_home(root .. "/stray/work", root .. "/stray/home", function() + return cwd.resolve({ cwd = "$HOME" }, {}) + end) + + assert.equals(realpath(root .. "/stray/home"), resolved and realpath(resolved)) + end) + + it("takes a static cwd with brackets and a star as the folder it names", function() + local repo = repo_in("setting", "[a] {b} *", function(path) + vim.fn.mkdir(path .. "/.git", "p") + end) + + local resolved = cwd.resolve({ cwd = repo }, {}) + + assert.equals(repo, resolved) + end) + end) +end) diff --git a/tests/mocks/vim.lua b/tests/mocks/vim.lua index bc53bc477..574f40ca6 100644 --- a/tests/mocks/vim.lua +++ b/tests/mocks/vim.lua @@ -705,6 +705,10 @@ local vim = { return "/home/user/project" end, + simplify = function(path) + return path + end, + --- Mock of wordcount() for the buffer that nvim_buf_call named, else buffer 1. Every line counts its bytes and a line --- break. A buffer with `empty = true` stands for Neovim's "--No lines in buffer--": its lines are { "" }, as --- nvim_buf_get_lines returns them, and it counts 0 bytes. diff --git a/tests/unit/cwd_spec.lua b/tests/unit/cwd_spec.lua index e7c07129c..a288dc19c 100644 --- a/tests/unit/cwd_spec.lua +++ b/tests/unit/cwd_spec.lua @@ -41,18 +41,22 @@ describe("The git root lookup", function() assert.same({ "git", "-C", dir, "rev-parse", "--show-toplevel" }, git_calls[1]) end) - it("runs git rev-parse once for a folder that is not in a repository, then searches for .git", function() + it("runs git rev-parse once for a folder that is not in a repository, then walks up to find a .git folder", function() stub_git({ lines = { "fatal: not a git repository" }, exit_code = 128 }) - local searched - vim.fn.finddir = function(name, path) - searched = { name = name, path = path } - return "" - end - - assert.is_nil(cwd.git_root(dir)) - + assert(os.execute("mkdir -p " .. temp_dir.quote(dir .. "/sub/deeper"))) + + -- A TMPDIR that sits inside a repository (a checkout, say) has a `.git` folder above `dir`, and the walk finds it. + -- So the lookup may find that repository, but never one inside `dir`. + local found = cwd.git_root(dir .. "/sub/deeper") + assert.is_true( + found == nil or found:sub(1, #dir) ~= dir, + "no repository inside " .. dir .. ", found " .. tostring(found) + ) assert.equals(1, git_runs, vim.inspect(git_calls)) - assert.same({ name = ".git", path = dir .. ";" }, searched) + + assert(os.execute("mkdir -p " .. temp_dir.quote(dir .. "/.git"))) + assert.equals(dir, cwd.git_root(dir .. "/sub/deeper")) + assert.equals(2, git_runs, vim.inspect(git_calls)) end) it("runs git rev-parse as a shell string, once, when systemlist refuses a list", function()