-
Notifications
You must be signed in to change notification settings - Fork 251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update remove_command.dart #834
base: main
Are you sure you want to change the base?
Conversation
remove with wildcard like this `dart run bin/main.dart remove '3.27.*'` Ref: leoafarias#833
@shinriyo is attempting to deploy a commit to the FlutterTools Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis change updates the version removal command to support wildcard patterns. When a user specifies a version string containing a wildcard ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant RC as RemoveCommand
participant C as Cache
participant L as Logger
U->>RC: Provide version string (possibly with wildcard)
RC->>RC: Check if version string contains "*"
alt Wildcard detected
RC->>C: Retrieve all available versions
C-->>RC: Return list of versions
RC->>RC: Filter versions using regex conversion
alt No matches found
RC->>L: Log informational message
RC-->>U: Return success code
else Matches found
RC->>U: Prompt for confirmation to remove matches
alt User declines
RC-->>U: Return success code (no removal)
else User confirms
loop For each matching version
RC->>C: Remove version from cache
alt Exception occurs
RC->>L: Log failure message
RC->>RC: Rethrow exception
end
end
RC-->>U: Return success code
end
end
else
RC->>RC: Execute existing specific version removal logic
end
Poem
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:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/src/commands/remove_command.dart (3)
64-71
: Great implementation of wildcard pattern matching, but consider escaping regex special charactersThe regex pattern construction doesn't account for potential special characters in the version string that have special meaning in regex (like
.
,+
,(
,)
, etc.). This could lead to unexpected behavior if the version string contains such characters.Consider escaping all regex special characters before replacing the wildcard:
- final pattern = version.replaceAll('*', '.*'); + final pattern = RegExp.escape(version).replaceAll('\\*', '.*');
87-96
: Exception handling could be more specificThe current implementation catches all exceptions generically. Consider catching more specific exceptions to provide more tailored error messages.
try { CacheService.fromContext.remove(matchingVersion); progress.complete('${matchingVersion.name} removed.'); - } on Exception { + } on FileSystemException catch (e) { - progress.fail('Could not remove ${matchingVersion.name}'); + progress.fail('Could not remove ${matchingVersion.name}: ${e.message}'); rethrow; + } on Exception catch (e) { + progress.fail('Error removing ${matchingVersion.name}: ${e.toString()}'); + rethrow; }
63-99
: Consider adding a helper method for reused removal logicI notice there's duplicated code between the wildcard pattern removal logic and the specific version removal logic (lines 111-121). Consider extracting the common functionality into a helper method to improve maintainability.
You could create a helper method like:
Future<void> _removeVersion(FlutterVersion version) async { final progress = logger.progress('Removing ${version.name}...'); try { CacheService.fromContext.remove(version); progress.complete('${version.name} removed.'); } on Exception { progress.fail('Could not remove ${version.name}'); rethrow; } }Then use this method in both the wildcard and specific version removal sections.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/src/commands/remove_command.dart
(1 hunks)
🔇 Additional comments (3)
lib/src/commands/remove_command.dart (3)
78-85
: Good user confirmation prompt for bulk removalThe confirmation prompt is clear and includes the count of matching versions, which is helpful for users to understand the impact of their action. Setting
defaultValue: false
is a good safety measure to prevent accidental deletions.
73-76
: Clear user feedback when no matching versions foundGood implementation of informative feedback when no matching versions are found.
64-99
: Overall well-implemented feature with good user experienceThe implementation of wildcard pattern matching for version removal is well thought out. It properly:
- Detects wildcard patterns
- Matches versions using regex
- Provides clear user feedback
- Requests confirmation before bulk actions
- Handles errors appropriately
This enhancement significantly improves the usability of the remove command.
remove with wildcard
like this
dart run bin/main.dart remove '3.27.*'
Ref:
#833
Summary by CodeRabbit