Floating window option #42
Conversation
When Claude Code runs inside Neovim, the NVIM environment variable points to a socket path instead of the nvim executable. This caused the test script to fail with permission errors. The fix checks if NVIM is an executable file (not a socket) before using it, otherwise falls back to finding nvim in PATH. This allows tests to run properly both from within Claude Code/Neovim and from regular terminals. Added: - Test script to verify NVIM detection logic works correctly - Documentation in DEVELOPMENT.md explaining the behavior
- Replace 'which' with POSIX-compliant 'command -v' in test scripts - Fix markdown formatting issues in DEVELOPMENT.md (add blank lines) - Separate variable declaration and assignment in test_nvim_detection.sh - Add robust absolute path resolution using readlink and BASH_SOURCE - Enhance error handling with 'set -euo pipefail' for strict mode - Replace unreliable $0 references in bash -c contexts These changes improve portability, reliability, and follow shell scripting best practices while addressing all linting and formatting issues.
WalkthroughFloating window support was introduced for the plugin's terminal interface. Configuration and documentation were updated to include floating window options, and a new module was added to manage floating window lifecycle and state. The terminal module now delegates to this floating window logic when the floating position is specified. Public APIs were extended accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Plugin
participant Terminal
participant Floating
User->>Plugin: Trigger terminal toggle
Plugin->>Plugin: Check config.window.position
alt position == "floating"
Plugin->>Floating: toggle(claude_code, config, git)
alt Floating window is open
Floating->>Floating: Close floating window
else Floating window is closed
Floating->>Floating: Create floating window & terminal
end
else position != "floating"
Plugin->>Terminal: toggle(claude_code, config, git)
Terminal->>Terminal: Open/close terminal in split window
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
lua/claude-code/config.lua (4)
17-24: Clarify & constrainbordertype
borderis validated asstring|table, but the docs don’t say what a “table” should look like (4-element style array? a map?). Consider tightening the spec or documenting the accepted table shape to avoid user confusion.
80-90: Defaultpositioncomment still implies split-only valuesThe comment string
"botright", "topleft", "vertical", "floating", etc.omits the existing"vsplit"/"rightbelow vsplit"values used elsewhere in the code. Aligning the doc with the actual parser would prevent mismatch between docs and behaviour.
174-204: Validation should be conditional onposition == "floating"Right now any config that happens to define
window.floatingis validated, even if the user chose a split position.
That means perfectly valid split configs that keep helper data underwindow.floatingcould fail.
Wrap the validation block in:if config.window.position == 'floating' then -- existing validation … endto avoid false-positives.
340-359: Avoid mutating the caller’suser_configtable
parse_configeditsuser_configin place (lines 341-351). Side-effects here can surprise callers who reuse the same table.
Safer pattern:- if user_config and user_config.floating then + local cfg = vim.deepcopy(user_config or {}) + if cfg.floating then … - user_config = cfg + user_config = cfg endlua/claude-code/init.lua (1)
25-45: Exportingfloatingtable is fine, but document public APIExternal plugins can now rely on
require('claude-code').floating. Please add a short note in the module header or README “Public API” section so future refactors don’t break consumers inadvertently.lua/claude-code/floating.lua (2)
21-29: Factor out common helper to avoid duplication
get_instance_identifieris byte-for-byte identical to the version interminal.lua. Consider moving it to a shared util module and re-using it from both places to keep behaviour in sync and reduce maintenance overhead.
34-41: Guard against headless UI / oversize ratios
vim.api.nvim_list_uis()[1]isnilin headless sessions, which will raise an error here.
Additionally, iffloating_config.widthorheightis set to a value ≥ 1, the calculated dimensions can exceed the screen and produce a negativerow/col. Please add:local ui = vim.api.nvim_list_uis()[1] or { width = 80, height = 24 } width = math.min(ui.width, math.floor(ui.width * floating_config.width)) height = math.min(ui.height, math.floor(ui.height * floating_config.height))or clamp via
math.max/min.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(2 hunks)lua/claude-code/config.lua(4 hunks)lua/claude-code/floating.lua(1 hunks)lua/claude-code/init.lua(3 hunks)lua/claude-code/keymaps.lua(1 hunks)lua/claude-code/terminal.lua(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lua/claude-code/floating.lua (1)
lua/claude-code/terminal.lua (1)
get_instance_identifier(23-31)
🔇 Additional comments (3)
lua/claude-code/keymaps.lua (1)
25-26: No functional impact – blank line onlyNothing to comment on here.
README.md (1)
96-106: Good doc updateDocs accurately reflect the new floating options.
lua/claude-code/terminal.lua (1)
103-107: State divergence when delegating to floating windows
terminal.toggleexits early for floating, but it never sets
claude_code.claude_code.current_instancenor stores the buffer in
claude_code.claude_code.instances.
Down-stream logic (e.g.keymaps.setup_terminal_navigation,force_insert_mode) expects those fields to be populated and will silently fail for floating windows.Request: ensure
floating.togglepopulates / updates the same state table or update consumers to look atclaude_code.floating.
| vim.api.nvim_create_autocmd({ 'WinClosed' }, { | ||
| buffer = bufnr, | ||
| callback = function() | ||
| if M.floating.instances[instance_id] then | ||
| M.floating.instances[instance_id].winid = nil | ||
| end | ||
| end, | ||
| once = true, | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
once = true prevents cleanup on subsequent windows
The WinClosed autocommand fires only for the first window created from this buffer; when the user re-opens and later manually closes the floating window again, winid is left stale, causing is_open() to mis-report until the next toggle. Drop once = true or recreate the autocommand each time you open a new window.
🤖 Prompt for AI Agents
In lua/claude-code/floating.lua around lines 189 to 197, the WinClosed
autocommand uses once = true, which causes it to run only once and not clean up
the winid on subsequent window closures. To fix this, remove the once = true
option so the autocommand triggers every time the floating window is closed,
ensuring the winid is properly cleared each time.
| local cmd = config.command | ||
| if config.git and config.git.use_git_root then | ||
| local git_root = git.get_git_root() | ||
| if git_root then | ||
| -- Use pushd/popd to change directory | ||
| local separator = config.shell.separator | ||
| local pushd_cmd = config.shell.pushd_cmd | ||
| local popd_cmd = config.shell.popd_cmd | ||
| cmd = pushd_cmd | ||
| .. ' ' | ||
| .. git_root | ||
| .. ' ' | ||
| .. separator | ||
| .. ' ' | ||
| .. config.command | ||
| .. ' ' | ||
| .. separator | ||
| .. ' ' | ||
| .. popd_cmd | ||
| end | ||
| end |
There was a problem hiding this comment.
Shell-command construction is vulnerable to spaces / quoting
Building cmd with string concatenation will break when git_root or config.command contain spaces or shell-metacharacters and can even be exploited if filenames are crafted maliciously. Please quote/escape the segments or, better, use vim.fn.termopen({cmd, arg1, …}, opts) with the list-form to avoid a shell entirely.
🤖 Prompt for AI Agents
In lua/claude-code/floating.lua around lines 148 to 168, the construction of the
shell command string using concatenation is unsafe because it does not handle
spaces or special shell characters in git_root or config.command. To fix this,
avoid building a single shell command string; instead, use vim.fn.termopen with
a list of command and arguments to bypass the shell and handle escaping
automatically. Refactor the code to pass the command and its arguments as a list
rather than a concatenated string.
|
We've just merged in another large floating window PR, and it brought up some merge issues here. This might also be duplication at this point. Can you take a look at the now merged PR and see if this shoiuld be reworked or closed? |
Pull Request
Description
This pull request introduces floating window support to claude-code.nvim, enabling users to open the Claude Code terminal in a floating Neovim window. The implementation adds a new
position = "floating"option to the window configuration, along with customizable floating window settings such as width, height, and border style.Type of Change
Please check the options that are relevant:
Checklist
Please check all that apply:
Screenshots (if applicable)
Summary by CodeRabbit