fix(v3): fix macOS mkdir brace expansion when APP_NAME contains spaces - #4850
Conversation
|
Warning Rate limit exceeded@leaanthony has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 58 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. 📒 Files selected for processing (3)
WalkthroughUpdates macOS Taskfile directory creation to use two explicit mkdir commands, adds an integration test for creating bundles when paths contain spaces, and documents the fix in the unreleased changelog. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Deploying wails with
|
| Latest commit: |
beca817
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e51ebe00.wails.pages.dev |
| Branch Preview URL: | https://fix-darwin-mkdir-brace-expan.wails.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v3/UNRELEASED_CHANGELOG.mdv3/internal/commands/build_assets/darwin/Taskfile.ymlv3/internal/commands/task_integration_test.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T22:11:37.054Z
Learnt from: leaanthony
Repo: wailsapp/wails PR: 3748
File: v3/internal/templates/_common/Taskfile.tmpl.yml:18-18
Timestamp: 2024-10-08T22:11:37.054Z
Learning: In Taskfile, variables are referenced using `{{ "{{variable}}" }}` without the dot notation.
Applied to files:
v3/internal/commands/build_assets/darwin/Taskfile.yml
🧬 Code graph analysis (1)
v3/internal/commands/task_integration_test.go (1)
v3/internal/commands/task.go (2)
RunTask(51-185)RunTaskOptions(24-49)
⏰ 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). (6)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Analyze (go)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
v3/UNRELEASED_CHANGELOG.md (1)
38-38: LGTM!Clear and concise changelog entry that describes both the symptom (app bundle creation failing with spaces in APP_NAME) and the root cause (brace expansion issue).
v3/internal/commands/build_assets/darwin/Taskfile.yml (2)
141-141: LGTM!The fix correctly addresses the brace expansion issue. By quoting path segments individually (
"{{.BIN_DIR}}"/"{{.APP_NAME}}") and leaving the brace expansion{MacOS,Resources}outside quotes, the shell can now properly expand the braces while still protecting spaces in variable values.
161-161: LGTM!Consistent application of the fix to the dev app bundle creation in the
runtask.v3/internal/commands/task_integration_test.go (2)
291-298: Well-documented test with clear purpose.The comment effectively explains the shell quoting behavior being tested and why the fix works. The CI skip condition follows the existing pattern in this file.
300-322: Good test setup with representative data.Using
"My App"with a space in the name directly exercises the bug scenario. The Taskfile structure mirrors the production fix, which makes this an effective regression test.
ea9c8a3 to
ca66f77
Compare
Replace brace expansion {MacOS,Resources} with two separate mkdir commands.
Brace expansion doesn't work inside quoted strings and is shell-dependent.
Adds integration test to verify mkdir works with spaces in paths.
ca66f77 to
5dbfb91
Compare
|
wailsapp#4850) fix(v3): fix macOS mkdir when APP_NAME contains spaces Replace brace expansion {MacOS,Resources} with two separate mkdir commands. Brace expansion doesn't work inside quoted strings and is shell-dependent. Adds integration test to verify mkdir works with spaces in paths.
1 participant
Summary
APP_NAMEcontains spaces{MacOS,Resources}doesn't work inside quoted strings, creating a literal{MacOS,Resources}directoryChanges
v3/internal/commands/build_assets/darwin/Taskfile.yml: Fix quoting increate:app:bundleandruntasksv3/internal/commands/task_integration_test.go: Add integration test to verify brace expansion works with spaces in pathsContext
Reported in #4845 (comment)
Type of change
How Has This Been Tested?
TestBraceExpansionWithSpacesInPathverifies:MacOSandResourcesdirectories{MacOS,Resources}directoryChecklist
v3/UNRELEASED_CHANGELOG.mdwith a description of my changesSummary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.