Persistent profiles #6
Conversation
|
Warning Rate limit exceeded@pablopunk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduced in this pull request enhance the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 4
🧹 Outside diff range and nitpick comments (3)
README.md (1)
244-261: Consider adding information about profile storage location.The documentation clearly explains the feature and provides good examples. Consider adding a note about where the profile information is stored (
.git/dotor.dotfile) to help users understand the persistence mechanism.🧰 Tools
🪛 Markdownlint (0.35.0)
259-259: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
spec/dot_spec.lua (1)
548-588: Enhance test coverage for profile persistence.While the test case effectively verifies the basic profile saving functionality, consider adding tests for:
- Profile removal using
--remove-profile- Edge cases like empty profile names or invalid profiles
- Different storage locations (
.git/dotvs.dot)Example test cases to add:
it("should remove the last used profile", function() -- Set up and save a profile first setup_profile( "test_profile", [[return { modules = { "neovim" } }]] ) assert.is_true(run_dot "test_profile") -- Verify profile is saved local dot_file_path = pl_path.join(dotfiles_dir, ".dot") assert.is_true(pl_path.isfile(dot_file_path)) -- Remove profile assert.is_true(run_dot "--remove-profile") -- Verify profile is removed assert.is_false(pl_path.isfile(dot_file_path)) end) it("should handle git directory for profile storage", function() -- Set up git directory local git_dir = pl_path.join(dotfiles_dir, ".git") pl_dir.makepath(git_dir) -- Save profile assert.is_true(run_dot "test_profile") -- Verify profile is saved in .git/dot local dot_file_path = pl_path.join(git_dir, "dot") assert.is_true(pl_path.isfile(dot_file_path)) end)dot.lua (1)
99-101: Consider using a consistent message format.The
print_sectionfunction introduces a new message format. Consider using the existingprint_messagefunction for consistency.-local function print_section(message) - print(colors.bold .. colors.blue .. "[" .. message .. "]" .. colors.reset) -end +local function print_section(message) + print("") + print_message("section", message) +end -- Update print_message to handle sections local function print_message(message_type, message) local color, symbol if message_type == "success" then color, symbol = colors.green, "✓" elseif message_type == "error" then color, symbol = colors.red, "✗" elseif message_type == "warning" then color, symbol = colors.yellow, "!" elseif message_type == "info" then color, symbol = colors.blue, "•" + elseif message_type == "section" then + print(colors.bold .. colors.blue .. "[" .. message .. "]" .. colors.reset) + return else color, symbol = colors.reset, ">" end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
README.md(2 hunks)dot.lua(11 hunks)spec/dot_spec.lua(1 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
README.md
259-259: null
Dollar signs used before commands without showing output
(MD014, commands-show-output)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
dot.lua (4)
20-20: Consistency: Initializeremove_profilewith other flagsFor better readability and consistency, consider grouping the initialization of
remove_profilewith other flag variables (e.g.,force_mode,purge_mode, etc.) at lines 13-19.This helps maintain a logical structure and makes it easier to locate related variables.
46-47: Validate command-line options inparse_argsThe
parse_argsfunction is becoming lengthy with multipleelseifconditions. Consider refactoring it to improve maintainability and readability.You could use a table to map command-line arguments to actions or flags, which would simplify adding new options in the future.
63-63: Update help message formattingThe help message for
--remove-profileis not aligned with the rest of the options. Consider adjusting the indentation for consistency.Apply this diff to fix the alignment:
--hooks Run hooks even if dependencies haven't changed - --remove-profile Remove the last used profile + --remove-profile Remove the last used profile -h Display this help message
99-101: Use existingprint_messagefunction for consistencyInstead of creating a new
print_sectionfunction, consider using the existingprint_messagefunction to maintain consistency in output formatting.If a distinct section header is necessary, perhaps extend
print_messageto handle this case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
dot.lua(11 hunks)
🔇 Additional comments (8)
dot.lua (8)
83-83: Ensure options table includes all parsed flags
Good job adding remove_profile to the returned options table. This ensures that the main function can access the new flag.
747-747: Enhance module processing messages
Using print_section(module_name) improves the readability of the script's output by clearly delineating module processing sections.
810-816: Handle loading errors in process_tool function
Good job adding error messages when loading or executing profiles fails. This will aid in debugging.
847-847: Handle missing modules gracefully
The error message "Module not found" helps users understand when a specified module does not exist.
852-862: Add error handling for file operations in save_last_profile
The save_last_profile function should handle potential errors when opening and writing to the file to prevent silent failures.
This issue was previously raised. Consider implementing error handling as suggested.
864-876: Add error handling and profile validation in get_last_profile
The get_last_profile function should validate that the profile exists and handle errors reading the file.
This mirrors a previous suggestion. Implementing validation ensures robustness.
878-884: Add error handling for file removal in remove_last_profile
The remove_last_profile function should check if the file exists before attempting to remove it and handle any errors during removal.
Refer to the prior feedback for detailed recommendations.
904-909:
Check the success of profile removal
After calling remove_last_profile, verify that the profile was successfully removed before confirming to the user.
Modify the function to return a status and update the main function accordingly.
Apply this diff:
if options.remove_profile then
- remove_last_profile()
- print_message("info", "Profile removed.")
+ local success = remove_last_profile()
+ if success then
+ print_message("info", "Profile removed.")
+ else
+ print_message("error", "Failed to remove profile.")
+ end
return
endAnd update remove_last_profile:
local function remove_last_profile()
local file_path = ".git/dot"
if not is_dir(".git") then
file_path = ".dot"
end
- os.remove(file_path)
+ local success, err = os.remove(file_path)
+ if not success then
+ return false
+ end
+ return true
endLikely invalid or redundant comment.
Summary by CodeRabbit
New Features
dotcommand.--remove-profileoption.Documentation
Tests