Properly handle the -f flag to be the primary file when used by up or down #36
Conversation
…pected behavior of the -f flag
|
Thank you so much! I appreciate your disclosure of using claude and I will review it when I have a chance |
|
I'm really excited about the possibilities here (for compose, which I think is sorely needed for |
There was a problem hiding this comment.
Pull request overview
This PR fixes the issue where the -f flag was being ignored when one of the default compose file names existed in the current directory. The changes ensure that when a user explicitly specifies a compose file via the -f flag, that file takes precedence over the automatic file discovery.
Key Changes
- Changed
composeFilenamefrom a non-optional String with a default value to an optional String (nil by default) - Modified the file discovery logic to only search for default filenames when
-fis not provided - Added support for absolute paths (starting with
/or~) in the-fflag
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| Sources/Container-Compose/Commands/ComposeUp.swift | Updated to respect the -f flag and handle absolute paths |
| Sources/Container-Compose/Commands/ComposeDown.swift | Updated to respect the -f flag and handle absolute paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if composeFilename == nil { | ||
| composeFilename = "compose.yml" | ||
| } |
There was a problem hiding this comment.
This fallback assignment is redundant since line 49 already handles the nil case with composeFilename ?? \"compose.yml\". The composePath computed property will always default to "compose.yml" if composeFilename is nil, making this assignment unnecessary.
| if composeFilename == nil { | |
| composeFilename = "compose.yml" | |
| } | |
| // No need to assign composeFilename = "compose.yml" here; composePath already handles the nil case. |
There was a problem hiding this comment.
Copilot is correct. this final if composeFilename == nil { is unnecessary. Please delete the if statement and do not leave the comment that copilot is suggesting.
| if composeFilename == nil { | ||
| composeFilename = "compose.yml" | ||
| } |
There was a problem hiding this comment.
Copilot is correct. this final if composeFilename == nil { is unnecessary. Please delete the if statement and do not leave the comment that copilot is suggesting.
| } | ||
| } | ||
| // If no file was found, default to compose.yml (will fail later with proper error) | ||
| if composeFilename == nil { |
There was a problem hiding this comment.
Copilot was correct. this final if composeFilename == nil { is unnecessary. Please delete the if statement and do not leave the comment that copilot is suggesting.
2 participants
As mentioned in #33, if you have one of the "key" compose file names,
upanddownwill choose that file, even if you've provided a file via the-fflag. This changes the logic to choose and handle the-fflag appropriately.With Swift not being my primary language, I've worked with Claude Code to build this PR.