default disabled compose stacks#71
Conversation
WalkthroughThis PR adds internationalization strings for Compose stack management (13 new keys across English, Spanish, and French), extends the DockerContainer model with compose working directory and config files fields, updates the Docker repository to fetch additional Compose labels, and implements a comprehensive Compose stack UI feature in the containers screen with stack action handlers, management cards, and file navigation, plus a new settings toggle for controlling stack card visibility. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 0
🧹 Nitpick comments (2)
lib/presentation/screens/containers_screen.dart (1)
67-84: Consider reducing the frequency of preference reloading.
_loadUserPreferences()is called every 1 second via the server change detection loop. While the method has an early exit if the value hasn't changed (line 91-95), it still performs aSharedPreferences.getInstance()call on every tick, which involves async I/O.Consider either:
- Only reloading preferences when the screen gains focus (via
didChangeAppLifecycleStateor similar)- Increasing the polling interval for preference changes
- Moving preference reload to a less frequent cadence (e.g., every 5-10 seconds)
lib/domain/models/docker_container.dart (1)
157-159: Consider including new fields in toString for debugging.The
toStringmethod could optionally includecomposeWorkingDirandcomposeConfigFilesfor more complete debugging output.Apply this diff if enhanced debugging output is desired:
@override String toString() { - return 'DockerContainer(id: $id, image: $image, names: $names, status: $status, stack: $composeProject)'; + return 'DockerContainer(id: $id, image: $image, names: $names, status: $status, stack: $composeProject, workingDir: $composeWorkingDir, configFiles: $composeConfigFiles)'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
assets/i18n/en-US.json(2 hunks)assets/i18n/es.json(2 hunks)assets/i18n/fr-FR.json(2 hunks)lib/data/repositories/docker_repository_impl.dart(1 hunks)lib/domain/models/docker_container.dart(6 hunks)lib/presentation/screens/containers_screen.dart(8 hunks)lib/presentation/screens/settings_screen.dart(4 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: build
🔇 Additional comments (21)
lib/data/repositories/docker_repository_impl.dart (1)
62-62: LGTM!The extended
docker psformat correctly adds the two new Compose labels (project.working_dirandproject.config_files) to enable stack management features. The pipe-delimited format is consistent with existing fields.assets/i18n/fr-FR.json (1)
152-163: LGTM!French translations for the new Compose stack management keys are complete and consistent with the English source. Placeholders are correctly preserved.
Also applies to: 302-303
assets/i18n/es.json (1)
152-163: LGTM!Spanish translations for the new Compose stack management keys are complete and consistent with the English source.
Also applies to: 302-303
assets/i18n/en-US.json (1)
152-163: LGTM!New i18n keys for Compose stack management are well-structured and follow existing naming conventions. The placeholder patterns (
{}) are appropriately used for dynamic values.Also applies to: 302-303
lib/presentation/screens/settings_screen.dart (3)
24-24: Verify the intended default value for_showStackCards.The PR title mentions "default disabled compose stacks", but the code defaults
_showStackCardstotrueboth at initialization (line 24) and when loading preferences (line 49). If the intent is to have stack cards disabled by default, this should befalse.Also applies to: 49-49
135-141: LGTM!The save method correctly persists the toggle state to SharedPreferences with the same key used for loading.
413-423: LGTM!The SwitchListTile is properly wired to the state variable and save method. The UI placement within the Docker Configuration section is appropriate.
lib/presentation/screens/containers_screen.dart (8)
13-13: LGTM!New import and state variables are properly scoped for stack management functionality.
Also applies to: 31-32
87-96: LGTM!The preference loading method correctly guards against unnecessary state updates when the value hasn't changed.
293-322: LGTM!The
_buildStackInfosmethod correctly groups containers by their compose project and aggregates metadata (working directory, config files, fallback path) with appropriate null-safety handling.
428-431: Shell quoting implementation is correct.The pattern
'$escaped'with'replaced by'"'"'is a well-known shell escaping technique that handles single quotes within single-quoted strings by:
- Ending the single quote
- Adding a double-quoted single quote
- Resuming the single quote
This is safe for shell command construction.
433-444: LGTM!The helper maps action names to their localized past-tense labels correctly.
987-1014: LGTM!The stack management UI components are well-structured with proper state handling for in-progress actions and appropriate button disabling when busy. The horizontal scrollable layout is suitable for varying numbers of stacks.
Also applies to: 1016-1115
1361-1423: LGTM!The
_ComposeStackInfodata class is well-designed with:
- Clear separation of raw data from computed properties
- Robust null-safety in getters
- Logical fallback chain in
filesPathandprojectDirectory- Reasonable
canUseComposeCliheuristic (requires project directory or config files with absolute paths)
324-407: Shell quoting implementation is correct and handles edge cases properly.The
_shellQuotefunction correctly implements POSIX shell single-quote escaping by replacing'with'"'"'(close quote, escaped quote, open quote) and wrapping the result in single quotes. This approach safely handles all edge cases including empty strings, paths with special characters, and strings with multiple quotes. Container IDs in the fallback branch are safe since they are hex strings. The exception for empty container lists provides appropriate error handling.lib/domain/models/docker_container.dart (6)
11-12: LGTM: Clean field additions.The new compose metadata fields follow the existing pattern and are appropriately nullable.
32-33: LGTM: Constructor properly extended.The new parameters are correctly positioned and follow the established pattern for optional compose metadata.
67-68: LGTM: Fields correctly propagated in copyWithStats.The new fields are properly carried forward when copying with stats, maintaining consistency with other non-stats fields.
80-80: LGTM: Parsing logic is sound.The extraction of compose metadata from tokens 9 and 10 follows the existing pattern with appropriate boundary and emptiness checks.
Also applies to: 97-98
140-154: LGTM: Robust config file extraction.The parsing handles multiple separators, trims whitespace, and filters empty entries appropriately.
123-138: Clarify intent oflastSlash > 0check for root-level config files.The condition
lastSlash > 0excludes root-level compose config files (e.g.,/docker-compose.yml), returningnullinstead of/. This appears intentional to avoid returning root as a project path, but it's undocumented and untested. Either add a comment explaining why root paths are excluded, or verify if this should use>= 0to support root-level files.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.