Update README #29
Update README #29
Conversation
WalkthroughREADME retitled and extensively reorganized: badges removed; Table of Contents added; new sections (Why, Requirements, Install, Quick Start, Commands, Configuration, Keymap examples, Dev Container template, Troubleshooting, FAQ, Credits, License); install snippet updated to inline setup; README example corrected Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment Pre-merge checks✅ Passed checks (3 passed)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
README.md (4)
47-50: toggleterm.nvim is marked “Required” but described as optional belowMove it out of “Requirements” or mark as optional to avoid confusion.
Apply this diff:
- - **toggleterm.nvim** (for the integrated terminal UI) + - toggleterm.nvim (optional; enhances the integrated terminal UI)
161-163: Fix argument quoting in keymap examplesUse unquoted values for non-strings; keep quotes only for
cmd. Current examples show'40'and'vertical'.Apply this diff:
- { "<leader>De", ":DevcontainerExec direction='vertical' size='40'<CR>", desc = "DevContainer: exec (vsplit)" }, + { "<leader>De", ":DevcontainerExec direction=vertical size=40<CR>", desc = "DevContainer: exec (vsplit)" }, - { "<leader>Db", ":DevcontainerExec cmd='cd build && make'<CR>", desc = "DevContainer: build" }, + { "<leader>Db", ":DevcontainerExec cmd='cd build && make'<CR>", desc = "DevContainer: build" }, - { "<leader>Dt", ":DevcontainerExec cmd='cd build && make test' direction='horizontal'<CR>", desc = "DevContainer: test" }, + { "<leader>Dt", ":DevcontainerExec cmd='cd build && make test' direction=horizontal<CR>", desc = "DevContainer: test" },
70-72: Call out the known typo in option name explicitlyStrengthen the NOTE so users don’t “fix” it to
installand break config. Consider tracking a rename with a backward‑compatible alias.Apply this diff:
- -- NOTE: the current option name is spelled `dotfiles_intallCommand` in the plugin + -- NOTE: Known typo in current plugin: option is `dotfiles_intallCommand` (intall). + -- Do not change to `install` unless the plugin releases an alias/rename.- -- NOTE: option currently spelled `dotfiles_intallCommand` in code + -- NOTE: Known typo: currently spelled `dotfiles_intallCommand` in code. + -- A future release may add `dotfiles_installCommand` as an alias.If you want, I can open an issue/PR to add a deprecation alias in code.
Also applies to: 132-134
94-95: Clarify “press t” mapping scope“t in normal mode” conflicts with Vim’s built‑in motion. Specify it’s a buffer‑local mapping in the devcontainer terminal window.
Apply this diff:
-**Pro tip:** While a Dev Container task is running, press `t` in normal mode to toggle the terminal window. Use `:DevContainerToggle` to bring it back later. +**Pro tip:** While a Dev Container task is running, in the devcontainer terminal window press `t` (buffer‑local) to hide it. Use `:DevContainerToggle` to bring it back later.-Press `t` in normal mode during a running task to hide the window; `:DevContainerToggle` brings it back. +In the devcontainer terminal window, press `t` (buffer‑local) during a running task to hide the window; `:DevContainerToggle` brings it back.Also applies to: 213-215
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
doc/gifs/devcontainer-cli-description.gifis excluded by!**/*.gif
📒 Files selected for processing (1)
README.md(1 hunks)
🔇 Additional comments (4)
README.md (4)
7-7: Image asset present — no change required.
Found doc/gifs/devcontainer-cli-description.gif at /home/jailuser/git/doc/gifs/devcontainer-cli-description.gif.
100-109: Double-check devcontainer command behavior descriptions — implementations not foundSearch returned no matches for the README-listed commands (e.g., :DevcontainerUp, :DevcontainerConnect, :DevcontainerExec, :DevcontainerDown, :DevContainerToggle). Confirm the README matches the actual implementations or provide the file paths that register/implement these commands.
100-110: Confirm and standardize Devcontainer command casing to the actual registered namesREADME mixes
:Devcontainer*with:DevContainerToggle— standardize to whatever the plugin actually registers.I ran the provided search and found no matches. Enumerate user commands locally and update README.md (94-95, 100-110, 164-165) with the exact names/casing — example checks to run locally:
# Lua user-command registrations rg -n --type=lua -C2 'vim\.api\.nvim_create_user_command|nvim_create_user_command|create_user_command' # Vimscript / dynamic command defs rg -n --type=vim -C2 '^\s*command!? ' || rg -n -C2 'vim\.cmd\s*\(\s*["'\''].*:command'
81-82: Confirm NVIM ≥ 0.9 and no 0.10-only APIs used
Automated scan returned no matches — manually verify there are no 0.10-only API usages (examples: vim.system, vim.fs.*, nvim_open_win with title option, nvim_buf_add_highlight_ns, create_autocmd variants) and confirm README NVIM requirement and Dev Container CLI instructions are correct.
README.md lines 81–82 and 201–206.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
README.md (5)
44-49: toggleterm.nvim appears optional; mark it as recommended, not required.You note users can “still call the commands” without toggleterm, so listing it under Requirements is contradictory. Move it to a Recommended/Optional dependency.
Suggested tweak:
- - **toggleterm.nvim** (for the integrated terminal UI) + - (Optional) **toggleterm.nvim** — enables the integrated terminal UIAnd keep the Tip as is.
69-69: Tilde expansion inside the container may not occur. Prefer $HOME or an absolute path.If the path isn’t passed through a shell,
"~/dotfiles"can resolve literally.$HOME/dotfiles(as above) or an absolute container path is safer.
97-109: Command name casing is inconsistent (Devcontainer vs DevContainerToggle).*Ensure the documented command names exactly match the plugin’s defined commands and keep casing consistent across the README to avoid confusion.
If the canonical form uses “Devcontainer*”, consider renaming all occurrences to match, or vice‑versa. Please confirm the actual Ex command names exported by the plugin and I can prep a precise diff.
Also applies to: 153-165, 93-94
86-93: Clarify dotfiles behavior in Quick start.Step 3 implies dotfiles are always cloned/applied. That only happens if dotfiles options are configured. Add a qualifier to prevent user confusion.
Proposed tweak:
-3. Run `:DevcontainerUp` to build + start the container, clone and apply your dotfiles, and run your setup script inside the container. +3. Run `:DevcontainerUp` to build + start the container. If dotfiles are configured, it will clone/apply them and run your setup script inside the container.Also, the “press
tin normal mode” tip—clarify scope (only in the plugin’s terminal window vs globally) to avoid implying a global mapping.
171-191: Template: consider hinting at the container user/home path.Users often expect dotfiles to target
$HOME. A note like “Most images use user ‘vscode’ with home/workspacesor/home/vscode; adjust dotfiles paths accordingly” can save troubleshooting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(1 hunks)
🔇 Additional comments (3)
README.md (3)
116-143: “Defaults shown” — verify these values match the actual code defaults.If any differ from the implementation, users will get surprising behavior. Either (a) confirm they’re the true defaults, or (b) label the block as “example configuration” and link to the authoritative help.
If they’re examples, change the intro line to “Example configuration (see :h devcontainer-cli.nvim for defaults)”.
7-7: Verify the GIF path exists in the published repo/package.Broken media in READMEs is common after reorganizations. Please confirm
doc/gifs/devcontainer-cli-description.gifis present on the default branch.
28-39: Nice “Why/Problem/Solution” section.Clear, skimmable framing. Good addition.
This PR hopes to improve the README to make things clearer.
Fixes #16
Summary by CodeRabbit