Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678

Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged

Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom
sergi:fix-error-launching-app-with-spaces

Conversation

@sergi

@sergiSergi Mansilla (sergi) commented Dec 17, 2020•
edited by ghost
Loading

Copy link
Copy Markdown

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

@sergiSergi Mansilla (sergi) changed the title Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy) approved these changes Dec 18, 2020
View reviewed changes

@alloyEloy Durán (alloy) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏

Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍

PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.

@ghost

Deleted user (ghost) commented Dec 18, 2020•
edited by ghost
Loading

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@alloy

Eloy Durán (alloy) commented Dec 18, 2020

Copy link
Copy Markdown
Member

Btw, be sure to sign the CLA.

@sergi

Sergi Mansilla (sergi) commented Dec 18, 2020

Copy link
Copy Markdown
Author

Signed, thanks!

@sergi

Sergi Mansilla (sergi) commented Dec 18, 2020

Copy link
Copy Markdown
Author

Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?

@sergiSergi Mansilla (sergi) mentioned this pull request Dec 22, 2020
Closed
4 tasks
@alloy

Eloy Durán (alloy) commented Dec 28, 2020

Copy link
Copy Markdown
Member

Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.

@alloy
Eloy Durán (alloy) merged commit b192f9c into microsoft:masterDec 28, 2020
HeyImChris pushed a commit to HeyImChris/react-native-macos that referenced this pull request Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit to HeyImChris/react-native-macos that referenced this pull request Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit that referenced this pull request Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces - #678
Merged
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Dec 28, 2020
Merged
Enclose the app path in quotes, so that the open command doesn't split it if it contains spaces#678
Eloy Durán (alloy) merged 2 commits into
microsoft:masterfrom 
sergi:fix-error-launching-app-with-spaces
Conversation
@sergi
@sergiSergi Mansilla (sergi)
commented
Dec 17, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

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
@sergiSergi Mansilla (sergi)
changed the title
Enclose the app path in quotes, so that the open command doesn't split if it contains spacesEnclose the app path in quotes, so that the open command doesn't split it if it contains spacesDec 17, 2020
alloy
Eloy Durán (alloy)
approved these changes
Dec 18, 2020
View reviewed changes
@alloyEloy Durán (alloy)
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🙏
Perhaps even better would have been if we were to use execFile here, i.e. not spawn a shell at all, but this is a good iterative fix on its own 👍
PS: If you’d like to see this in a v0.63.x stable release, it would be great if you could send the same PR to the 0.63-stable branch.
@ghost
Deleted user (ghost)
commented
Dec 18, 2020•

edited by ghost

Loading

Copy link


Copy Markdown

CLA assistant check
All CLA requirements met.
@alloy
Eloy Durán (alloy)
commented
Dec 18, 2020

Copy link


Copy Markdown

Member
Btw, be sure to sign the CLA.
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Signed, thanks!
@sergi
Sergi Mansilla (sergi)
commented
Dec 18, 2020

Copy link


Copy Markdown

Author
Eloy Durán (@alloy) I see a homebrew-related fix has been commited to this PR. Is it related?
@sergiSergi Mansilla (sergi)
mentioned this pull request
Dec 22, 2020
 Closed

4 tasks
@alloy
Eloy Durán (alloy)
commented
Dec 28, 2020

Copy link


Copy Markdown

Member
Nope, that was to fix an earlier CI failure. I think this failure is a flunk, so I’m re-running the failed task.
@alloy
Eloy Durán (alloy)
merged commit b192f9c
into
microsoft:masterDec 28, 2020
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 24, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
5c858fc
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris pushed a commit
to HeyImChris/react-native-macos
that referenced
this pull request
Feb 26, 2021
@sergi@alloy@HeyImChris
Enclose the app path in quotes, so that the open command doesn't sp…
fc84678
…lit it if it contains spaces (microsoft#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
HeyImChris added a commit
that referenced
this pull request
Feb 26, 2021
@HeyImChris@acoates-ms
@sergi@alloy
Fix 0.63-stable CI, remove (#735)
21cfd79
* Add nullability checks (#704)
* Update RCTCxxBridge.mm
* add nullability checks
* CI should run on PRs to future stable branches (#718)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces (#678)
* Enclose the app path in quotes, so that the `open` command doesn't split it if it contains spaces
* [ado] Remove no longer needed homebrew fix
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
* update podfile.lock
* windows dep updates to 63
Co-authored-by: Andrew Coates <30809111+acoates-ms@users.noreply.github.com>
Co-authored-by: Sergi Mansilla <sergi@sergimansilla.com>
Co-authored-by: Eloy Durán <eloy.de.enige@gmail.com>
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


1 more reviewer

@alloyEloy Durán (alloy)alloy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@sergi@alloy