docs: replace unused dotfiles_installCommand config option with dotfiles_install_command - #37
Open
AlexTMjugador wants to merge 2 commits into
Open
docs: replace unused dotfiles_installCommand config option with dotfiles_install_command#37AlexTMjugador wants to merge 2 commits into
dotfiles_installCommand config option with dotfiles_install_command#37AlexTMjugador wants to merge 2 commits into
Conversation
…tfiles_install_command` Ever since documentation and devcontainer CLI launching code was reworked on erichlf@43ae866#diff-1b38a8285e8e71b146aa9a90a37fa3edf2d5d7ec1ccb76c21c0427b245c9c434R156, `devcontainer-cli.nvim` reads the install command value from the `dotfiles_install_command` config variable, instead of the `dotfiles_installCommand` variable documented everywhere else. This bug usually does not affect functionality because the devcontainer CLI [falls back to a list of well-known install commands](https://github.com/devcontainers/cli/blob/58be9705761d276b5076525438bbe73642f521d5/src/spec-common/dotfiles.ts#L11-L20) when an install command is not set explicitly. However, as soon as an attempt to use `dotfiles_installCommand` to set a non-conventional install command is made, the bug obviously surfaces after paying attention to the devcontainer CLI output. This change fixes the mismatch by updating the documentation to match the variable actually used in the code, and dropping the useless `dotfiles_installCommand` default from the `default_config` table, with a comment noting the devcontainer CLI fallback behavior the plugin was relying on. I have decided against modifying the code to match the documentation instead because I recognize that maybe I'm not the first person to notice this mismatch, and as such it's possible that there is some plugin configuration out there depending on the variable name that already works.
AlexTMjugador
changed the title
dotfiles_installCommand config option with `do…dotfiles_installCommand config option with dotfiles_install_command
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe dotfiles installation option now uses ChangesDotfiles option rename
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 131: Update the README entries for dotfiles_install_command to document
that its default is unset rather than "install.sh", and explain that omitting
the option allows the Dev Container CLI to select its well-known install
command. Apply the same correction to both referenced occurrences.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b3aa867b-2239-4af1-9f03-762501971a88
📒 Files selected for processing (2)
README.mdlua/devcontainer-cli/config.lua
Owner
|
@coderabbitai review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ever since documentation and devcontainer CLI launching code was reworked on
43ae866#diff-1b38a8285e8e71b146aa9a90a37fa3edf2d5d7ec1ccb76c21c0427b245c9c434R156,
devcontainer-cli.nvimreads the install command value from thedotfiles_install_commandconfig variable, instead of thedotfiles_installCommandvariable documented everywhere else.This bug usually does not affect functionality because the devcontainer CLI falls back to a list of well-known install
commands when an install command is not set explicitly. However, as soon as an attempt to use
dotfiles_installCommandto set a non-conventional install command is made, the bug obviously surfaces after paying attention to the devcontainer CLI output.This change fixes the mismatch by updating the documentation to match the variable actually used in the code, and dropping the useless
dotfiles_installCommanddefault from thedefault_configtable, with a comment noting the devcontainer CLI fallback behavior the plugin was relying on. I have decided against modifying the code to match the documentation instead because I recognize that maybe I'm not the first person to notice this mismatch, and as such it's possible that there is some plugin configuration out there depending on the variable name that already works.Summary by CodeRabbit
Bug Fixes
Documentation