Skip to content

Conversation

@sergi
Copy link

@sergi sergi commented Dec 22, 2020

(This PR has already been accepted for the masterbranch)

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

When running npx react-native run-macos (or locally node node_modules/react-native-macos/local-cli/cli.js run-macos) and if the target app has spaces in the name, the current call to the open macOS command will split the path and only keep the last part as the name in the app, which will fail:

...
▸ Build Succeeded
info Launching app "io.sergi.todolist" from "/Users/sergi/Library/Developer/Xcode/DerivedData/Todo_List-efowxgkzzzbxtgeghbplezporgmk/Build/Products/Debug/Todo List.app"
error Failed to launch the app, The file /Users/sergi/code/todo_list/macos/List.app does not exist.

This fix surrounds the path in quotes, which is the standard way to deal with spaces in the path. It solves the issue and introduces no other effects.

Changelog

[macOS] [Fixed] - Fixes CLI failing to launch app if the path contains spaces

Test Plan

This fix requires no further testing, as it only encloses a file path in quotes, keeping the exact same previous functionality, and preventing spaces from making open command misbehave.

Microsoft Reviewers: Open in CodeFlow

@sergi sergi requested a review from alloy as a code owner December 22, 2020 08:14
@pull-bot
Copy link

Warnings
⚠️

❔ Base Branch - The base branch for this PR is something other than master. Are you sure you want to merge these changes into a stable release? If you are interested in backporting updates to an older release, the suggested approach is to land those changes on master first and then cherry-pick the commits into the branch for that release. The Releases Guide has more information.

Generated by 🚫 dangerJS against e96a6d5

@sergi sergi closed this Dec 22, 2020
@sergi
Copy link
Author

sergi commented Dec 22, 2020

I guess I'll wait for the PR to be merged in master and then cherry-pick that one instead of making 2 PRs, as pull-bot suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants