Skip to content

Conversation

@thymikee
Copy link
Member

Summary:

It may happen, that the terminal detected by CLI or passed by the user is wrong and not really an executable. A good example is vscode, which sets TERM_PROGRAM env var to vscode, which is neither a command, nor a macOS app name. This PR introduces a fallback behavior for such cases, so that the terminal will be opened by a default open program. If that doesn't work, we produce a warning with directions on what to do now:

Screenshot 2019-07-31 at 21 50 31

Test Plan:

Packager should start in new terminal window, when running run-android from vscode or any other terminal.

description:
'Launches the Metro Bundler in a new window using the specified terminal path.',
default: getDefaultUserTerminal,
default: getDefaultUserTerminal(),
Copy link
Member

Choose a reason for hiding this comment

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

How was it working previously?

Copy link
Member Author

Choose a reason for hiding this comment

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

it either didn't or used open/sh to open with a default program

@thymikee thymikee merged commit 30a3023 into master Aug 1, 2019
@thymikee thymikee deleted the fix/open-terminal-vscode branch August 1, 2019 07:45
thymikee added a commit that referenced this pull request Aug 1, 2019
* origin/master:
  imp: optimize gradle code generation (#603)
  v2.8.2
  fix: corrects `srcDirs` to be compatible with Kotlin projects (#602)
  v2.8.1
  fix: fallback to default terminal if wrong one passed (#601)
  fix: adding back `projectRoot` to option (#588)
  fix: init run instructions to show workspace (#566)
@samuelgranja
Copy link

samuelgranja commented Aug 1, 2019

Hello @thymikee ,

When I see "Checks" tab it seems to me that built has failed :/
Could you confirm, please?

We are struggling with this issue since we upgraded RN to 0.60.4 and it is not running the packager unless we explicitly specify --terminal terminal

@thymikee
Copy link
Member Author

thymikee commented Aug 1, 2019

@samuelgranja please describe your issue in detail. Ideally create a new one, with a repro we can download and investigate, describe environment, etc. Otherwise, it's not really actionable.

@Jeangel
Copy link

Jeangel commented Aug 1, 2019

Hi @thymikee, why this approach is only implemented for android? With the new version 2.8.1/2.8.2 it works like a charm on android but in iOS it still uses vscode or Apple_Terminal. Could you confirm why only for android?

@thymikee
Copy link
Member Author

thymikee commented Aug 2, 2019

@Jeangel I guess one of the reasons may be that nobody sent a PR 😉

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.

5 participants