Refactor CLI and add migration command using dox_migration package - #111
Conversation
WalkthroughThis update introduces a major refactor and version bump across the Dox framework and its packages, centralizing CLI command handling via a new registry system, improving documentation, updating migration templates and error handling, and synchronizing version numbers using a new automation script. The CLI now uses structured command definitions and dynamic help, and package versions are unified at 3.0.0. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (dox.dart)
participant CommandRegistry
participant CommandFunction
User->>CLI (dox.dart): Run `dox <command> [args]`
CLI (dox.dart)->>CommandRegistry: findCommand(<command>)
alt Command found
CLI (dox.dart)->>CommandRegistry: validateArgs([args])
alt Args valid
CLI (dox.dart)->>CommandFunction: Execute asynchronously
CommandFunction-->>CLI (dox.dart): Completion / Output
else Args invalid
CLI (dox.dart)-->>User: Print argument error
end
else Command not found
CLI (dox.dart)-->>User: Print unknown command error
end
sequenceDiagram
participant User
participant CLI
participant CommandRegistry
User->>CLI: Run `dox help [command]`
CLI->>CommandRegistry: getAllCommands() / findCommand(command)
alt No argument
CLI-->>User: Print categorized command list
else With command argument
alt Command exists
CLI-->>User: Print detailed help for command
else
CLI-->>User: Print error, suggest general help
end
end
Poem
✨ Finishing Touches
🪧 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: 4
🧹 Nitpick comments (5)
packages/dox-cli/lib/src/tools/help.dart (1)
28-29: Handle long command help info gracefully.The padding calculation assumes
helpInfois less than 30 characters. If it exceeds this length, negative padding could cause formatting issues.Consider using a more robust formatting approach:
- final padding = ' ' * (30 - command.helpInfo.length); - print(' ${command.helpInfo}$padding ${command.description}'); + final padding = command.helpInfo.length < 30 + ? ' ' * (30 - command.helpInfo.length) + : ' '; + print(' ${command.helpInfo}$padding ${command.description}');packages/dox-cli/COMMAND_REGISTRY.md (1)
136-141: Add language specification to the code block.The fenced code block should specify the language for proper syntax highlighting.
-``` +```text Command: create:controller Usage: dox create:controller <controller_name> [-r] [-ws] Description: Create a new controller Arguments: 1 to 2 arguments</blockquote></details> <details> <summary>packages/dox-cli/test/command_registry_test.dart (1)</summary><blockquote> `7-19`: **Remove unused OutputCapture class.** The `OutputCapture` helper class is defined but never used in any of the tests. Consider removing this unused code or implementing output capture tests if needed for verifying command output. </blockquote></details> <details> <summary>packages/dox-cli/lib/src/command_registry.dart (2)</summary><blockquote> `187-187`: **Consider consistent categorization for create commands.** The `create:controller` command is categorized as "development" while `create:model` and `create:serializer` are in "generation". For consistency, consider placing all `create:*` commands in the same category. ```diff - category: CommandCategory.development, + category: CommandCategory.generation,This would also apply to
create:middleware(line 208) andcreate:request(line 221).
306-314: Simplify command lookup using firstWhereOrNull.The try-catch pattern around
firstWhereworks but is less idiomatic than usingfirstWhereOrNull.+import 'package:collection/collection.dart'; + static CommandDefinition? findCommand(String command) { - try { - return _commands.firstWhere( - (cmd) => cmd.matches(command), - ); - } catch (e) { - return null; - } + return _commands.firstWhereOrNull( + (cmd) => cmd.matches(command), + ); }Note: This requires adding
collectionpackage to dependencies if not already present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/dox-app/pubspec.lockis excluded by!**/*.lockpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.github/workflows/test.yaml(1 hunks).gitignore(1 hunks)compile_dox.sh(1 hunks)docs/digging-deeper/dox-cli.md(2 hunks)docs/others/contribute.md(1 hunks)packages/dox-cli/.env.example(1 hunks)packages/dox-cli/COMMAND_REGISTRY.md(1 hunks)packages/dox-cli/bin/dox.dart(1 hunks)packages/dox-cli/lib/src/command_registry.dart(1 hunks)packages/dox-cli/lib/src/tools/help.dart(1 hunks)packages/dox-cli/pubspec.yaml(1 hunks)packages/dox-cli/test/command_registry_test.dart(1 hunks)packages/dox-migration/bin/dox_migration.dart(0 hunks)packages/dox-migration/lib/src/create_migration/create_migration.dart(1 hunks)packages/dox-migration/lib/src/dox_migration_base.dart(1 hunks)packages/dox-migration/lib/src/utils/env.dart(1 hunks)
💤 Files with no reviewable changes (1)
- packages/dox-migration/bin/dox_migration.dart
🧰 Additional context used
🧠 Learnings (3)
.gitignore (1)
Learnt from: necessarylion
PR: dartondox/dox#110
File: packages/dox-app/.env.example:1-4
Timestamp: 2025-07-11T02:37:01.418Z
Learning: For the dox project, the team prefers to keep realistic-looking values in .env.example files rather than using placeholders, as they believe it helps show the expected format to developers.
.github/workflows/test.yaml (1)
Learnt from: necessarylion
PR: dartondox/dox#110
File: packages/dox-app/.env.example:1-4
Timestamp: 2025-07-11T02:37:01.418Z
Learning: For the dox project, the team prefers to keep realistic-looking values in .env.example files rather than using placeholders, as they believe it helps show the expected format to developers.
packages/dox-cli/.env.example (1)
Learnt from: necessarylion
PR: dartondox/dox#110
File: packages/dox-app/.env.example:1-4
Timestamp: 2025-07-11T02:37:01.418Z
Learning: For the dox project, the team prefers to keep realistic-looking values in .env.example files rather than using placeholders, as they believe it helps show the expected format to developers.
🪛 Gitleaks (8.26.0)
packages/dox-cli/.env.example
4-4: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.17.2)
packages/dox-cli/COMMAND_REGISTRY.md
136-136: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (11)
packages/dox-cli/pubspec.yaml (1)
13-13: LGTM! Dependency addition aligns with CLI refactor.The addition of
dox_migration: ^2.0.0correctly supports the integration of migration commands into the centralized command registry system..gitignore (1)
12-12: LGTM! Correctly ignores compiled CLI executable.Adding
doxto gitignore is appropriate for excluding compiled CLI executables from version control.packages/dox-migration/lib/src/utils/env.dart (1)
8-10: LGTM! Improved error handling for missing .env file.Adding the existence check before reading the .env file prevents runtime errors and provides clear feedback when the configuration file is missing.
docs/digging-deeper/dox-cli.md (2)
114-114: LGTM! Updated command syntax matches new CLI architecture.The change from
dox migratetodox migration:runcorrectly reflects the new centralized command registry system with namespace:action format.
131-131: LGTM! Rollback command syntax updated consistently.The change to
dox migration:rollbackmaintains consistency with the new command registry format..github/workflows/test.yaml (1)
62-65: LGTM! Workflow properly supports both packages.The split into separate steps for copying .env.example files is cleaner and properly supports the new CLI package environment configuration. The descriptive step names improve workflow clarity.
packages/dox-cli/.env.example (1)
1-15: Environment example file looks good.The configuration covers all necessary sections (app, database, Redis) with realistic-looking values, which aligns with the team's preference for example files. The static analysis warning about the API key is a false positive since this is an example file with a sample key.
docs/others/contribute.md (1)
25-48: Excellent documentation additions for contributors.The new "Other Useful Commands" section provides valuable guidance for contributors, including testing commands, CLI compilation instructions, and troubleshooting tips. This will help streamline the development experience.
packages/dox-migration/lib/src/dox_migration_base.dart (1)
38-51: Excellent error handling improvement.The addition of explicit error handling around the database connection attempt significantly improves the robustness of the migration system. The try-catch block provides clear error messages that will help developers diagnose connection issues more effectively.
packages/dox-cli/lib/src/tools/help.dart (1)
35-68: Well-structured command help implementation!The function properly handles command lookup, provides clear error messages, and includes thoughtful pluralization for argument display.
packages/dox-cli/bin/dox.dart (1)
1-41: Excellent refactoring to use the command registry pattern!The code is now much cleaner with proper separation of concerns, comprehensive error handling, and clear user feedback. The delegation to CommandRegistry centralizes command logic effectively.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
compile_dox.sh (1)
1-28: Add comprehensive error handling for all commands.The script has improved with basic validation but still lacks error handling for the core operations (dart compile, rm, cp), which could lead to silent failures.
Consider adding these improvements:
#!/bin/bash +set -e # Exit on any error PUBSPEC_FILE="packages/dox-cli/pubspec.yaml" VERSION_FILE="packages/dox-cli/lib/src/version.dart" # Check if pubspec.yaml exists if [ ! -f "$PUBSPEC_FILE" ]; then echo "Error: $PUBSPEC_FILE not found" exit 1 fi # Extract version from pubspec.yaml VERSION=$(grep "^version:" "$PUBSPEC_FILE" | sed 's/version:[[:space:]]*//' | tr -d '"' | tr -d "'") # Check if version was found if [ -z "$VERSION" ]; then echo "Error: Could not find version in $PUBSPEC_FILE" exit 1 fi # Update the version.dart file echo "const version = '$VERSION';" > "$VERSION_FILE" echo "Updated $VERSION_FILE with version: $VERSION" +echo "Compiling Dart CLI executable..." dart compile exe packages/dox-cli/bin/dox.dart -o packages/dox-cli/bin/dox +if [ $? -ne 0 ]; then + echo "Error: Failed to compile Dart executable" + exit 1 +fi + +echo "Removing old executable..." rm -rf packages/dox-app/bin/dox + +echo "Copying new executable..." cp packages/dox-cli/bin/dox packages/dox-app/bin/dox +if [ $? -ne 0 ]; then + echo "Error: Failed to copy executable" + exit 1 +fi + +echo "✅ CLI executable compiled and copied successfully!"packages/dox-cli/lib/src/command_registry.dart (1)
250-258: Fix inconsistency between help text and implementation.The help text indicates flags
[--sql|--dart], but the implementation treats it as a positional argument.Either update the help text or fix the implementation:
- helpInfo: 'create:migration <migration_name> [--sql|--dart]', + helpInfo: 'create:migration <migration_name> [sql|dart]',Or implement proper flag parsing:
function: (args) async { final migrationName = args[0]; - final type = args.length >= 2 ? args[1] : 'sql'; + String type = 'sql'; + if (args.length >= 2) { + if (args[1] == '--sql') type = 'sql'; + else if (args[1] == '--dart') type = 'dart'; + else type = args[1]; // fallback to positional + } MigrationFile(migrationName, type); },
🧹 Nitpick comments (2)
packages/dox-cli/lib/src/tools/help.dart (1)
36-69: Consider extracting formatting constants for better maintainability.The command-specific help function works well, but the ANSI escape codes are scattered throughout the code.
Consider extracting formatting constants:
+// At the top of the file +const String _bold = '\x1B[1m'; +const String _reset = '\x1B[0m'; + helpCommand(String commandName) { final command = CommandRegistry.findCommand(commandName); if (command == null) { print('Command "$commandName" not found.'); print('Run "dox help" to see all available commands.'); return; } - print('\x1B[1mCommand:\x1B[0m ${command.command}'); - print('\x1B[1mUsage:\x1B[0m dox ${command.helpInfo}'); - print('\x1B[1mDescription:\x1B[0m ${command.description}'); - print('\x1B[1mCategory:\x1B[0m ${command.category.displayName}'); + print('${_bold}Command:$_reset ${command.command}'); + print('${_bold}Usage:$_reset dox ${command.helpInfo}'); + print('${_bold}Description:$_reset ${command.description}'); + print('${_bold}Category:$_reset ${command.category.displayName}'); if (command.aliases.isNotEmpty) { - print('\x1B[1mAliases:\x1B[0m ${command.aliases.join(', ')}'); + print('${_bold}Aliases:$_reset ${command.aliases.join(', ')}'); } if (command.minArgs > 0 || command.maxArgs != -1) { String argsInfo = ''; if (command.minArgs == command.maxArgs) { argsInfo = '${command.minArgs} argument${command.minArgs == 1 ? '' : 's'}'; } else if (command.maxArgs == -1) { argsInfo = 'at least ${command.minArgs} argument${command.minArgs == 1 ? '' : 's'}'; } else { argsInfo = '${command.minArgs} to ${command.maxArgs} arguments'; } - print('\x1B[1mArguments:\x1B[0m $argsInfo'); + print('${_bold}Arguments:$_reset $argsInfo'); } print(''); }packages/dox-cli/COMMAND_REGISTRY.md (1)
168-168: Add language specification to fenced code block.The static analysis tool correctly identified a fenced code block missing language specification.
-``` +```text Command: create:controller Usage: dox create:controller <controller_name> [-r] [-ws] Description: Create a new controller Category: Generation Arguments: 1 to 2 arguments</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between dfa8c952482927fdb21e4d98185585989e73e011 and e14b51a1311599e9b0262a932164042353906943. </details> <details> <summary>📒 Files selected for processing (8)</summary> * `compile_dox.sh` (1 hunks) * `packages/dox-cli/COMMAND_REGISTRY.md` (1 hunks) * `packages/dox-cli/lib/src/command_registry.dart` (1 hunks) * `packages/dox-cli/lib/src/tools/help.dart` (1 hunks) * `packages/dox-cli/lib/src/types.dart` (1 hunks) * `packages/dox-cli/lib/src/version.dart` (1 hunks) * `packages/dox-cli/test/command_registry_test.dart` (1 hunks) * `packages/dox-migration/lib/src/create_migration/create_migration.dart` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (3)</summary> * packages/dox-cli/lib/src/version.dart * packages/dox-migration/lib/src/create_migration/create_migration.dart * packages/dox-cli/lib/src/types.dart </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * packages/dox-cli/test/command_registry_test.dart </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>packages/dox-cli/COMMAND_REGISTRY.md</summary> 168-168: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)</summary> * GitHub Check: test </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>packages/dox-cli/lib/src/tools/help.dart (1)</summary> `1-34`: **LGTM! Well-structured dynamic help system.** The refactoring to use CommandRegistry for dynamic help generation is excellent. The code properly organizes commands by category, sorts them appropriately, and provides clear formatting. </details> <details> <summary>packages/dox-cli/COMMAND_REGISTRY.md (1)</summary> `1-209`: **Excellent comprehensive documentation!** The documentation is thorough, well-structured, and provides clear guidance for developers. It covers all aspects of the command registry system with practical examples and clear explanations. </details> <details> <summary>packages/dox-cli/lib/src/command_registry.dart (2)</summary> `42-44`: **Version is now dynamically managed - issue resolved.** The version is now imported from the `version.dart` file, which is dynamically updated by the `compile_dox.sh` script. This resolves the hardcoded version concern. --- `263-280`: **LGTM! Well-designed registry methods.** The static methods for finding commands, retrieving all commands, and adding commands are well-implemented with proper error handling. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
bin/update_versions.sh (2)
76-93: Duplicate logic for version extraction.This section duplicates the version extraction logic from lines 22-31. The code could be more maintainable by extracting this into a function.
Consider extracting version extraction into a function:
+# Function to extract version from pubspec.yaml +extract_version() { + local pubspec_file="$1" + if [ ! -f "$pubspec_file" ]; then + echo "Error: $pubspec_file not found" >&2 + return 1 + fi + + local version=$(grep "^version:" "$pubspec_file" | sed 's/version:[[:space:]]*//' | tr -d '"' | tr -d "'") + if [ -z "$version" ]; then + echo "Error: Could not find version in $pubspec_file" >&2 + return 1 + fi + + echo "$version" +}Then use it in both places:
-VERSION=$(grep "^version:" pubspec.yaml | sed 's/version: //' | tr -d ' ') +VERSION=$(extract_version "pubspec.yaml")
24-31: Add validation for extracted version format.The script doesn't validate that the extracted version follows semantic versioning format, which could cause issues downstream.
Add version format validation:
# Check if version was found if [ -z "$VERSION" ]; then echo -e "${RED}Error: Could not extract version from pubspec.yaml${NC}" exit 1 fi + +# Validate version format (basic semver check) +if ! echo "$VERSION" | grep -E '^[0-9]+\.[0-9]+\.[0-9]+' > /dev/null; then + echo -e "${RED}Error: Invalid version format: $VERSION${NC}" + echo -e "${RED}Expected semantic versioning format (e.g., 1.0.0)${NC}" + exit 1 +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)bin/update_versions.sh(1 hunks)packages/dox-cli/lib/src/version.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .gitignore
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
packages/dox-cli/lib/src/version.dart (1)
1-1: LGTM! Clean and straightforward version constant definition.The version constant is properly defined and follows semantic versioning format. This will be useful for the CLI to provide version information.
bin/update_versions.sh (1)
6-6: Good use of strict error handling.The
set -eflag ensures the script exits on any error, which is a good practice for automation scripts.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
bin/update_versions.sh (3)
24-24: Standardise version-extraction regex to avoid subtle parsing bugs
Using two differentsedpatterns ('s/version: //'vs's/version:[[:space:]]*//') was flagged in the previous review and is still present. Pick a single, whitespace-tolerant pattern everywhere.-# Line 24 -VERSION=$(grep "^version:" pubspec.yaml | sed 's/version: //' | tr -d ' ') +VERSION=$(grep "^version:" pubspec.yaml | sed 's/version:[[:space:]]*//' | tr -d '"' | tr -d "'") -# Line 71 -CURRENT_VERSION=$(grep "^version:" "$pubspec_file" | sed 's/version: //' | tr -d ' ') +CURRENT_VERSION=$(grep "^version:" "$pubspec_file" | sed 's/version:[[:space:]]*//' | tr -d ' ')Also applies to: 70-71
49-56: Backup handling still duplicates files and leaves clutter
You now create two backups (.backupand the.bakfromsed -i.bak) but only delete one. Either rely on thesedbackup exclusively or clean up both afterwards:- # Create a backup of the original file - cp "$pubspec_file" "${pubspec_file}.backup" + # Rely on sed's .bak backup – no extra .backup copy needed…and/or append a cleanup loop after the summary to delete any
*.backupfiles if you really need them only temporarily.
83-86: Still no error handling around compile / copy steps
The earlier review requested explicit checks so failures don’t pass silently or leave the tree half-updated.set -estops execution, but you’ll lose context on why it failed. Wrap these calls and emit explicit messages for easier CI troubleshooting.
🧹 Nitpick comments (1)
bin/update_versions.sh (1)
83-83: Colour code is never reset – subsequent output stays green
Terminate the colour sequence with${NC}:-echo -e "${YELLOW}Compiling Dox CLI...${GREEN}" +echo -e "${YELLOW}Compiling Dox CLI...${NC}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bin/update_versions.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Tests
Revert