Skip to content

Remove open-in flag #2013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 27, 2020
Merged

Remove open-in flag #2013

merged 1 commit into from
Aug 27, 2020

Conversation

code-asher
Copy link
Member

@code-asher code-asher commented Aug 27, 2020 •
edited
Loading

  • Always launch into the existing instance if there is a socket
    • VS Code sets this variable when the socket is created so if this is set it should mean the socket exists
  • Unset for yarn watch so you can still develop code-server inside code-server
    • Otherwise it would just send to the socket instead of launching
  • Avoid the config info line when sending to an existing instance

@code-asher code-asher requested a review from nhooyr as a code owner August 27, 2020 18:41
@code-asher code-asher mentioned this pull request Aug 27, 2020
Merged
nhooyr
nhooyr approved these changes Aug 27, 2020
View reviewed changes
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

🚀

package.json
@@ -26,7 +26,7 @@
"lint": "./ci/dev/lint.sh",
"test": "./ci/dev/test.sh",
"ci": "./ci/dev/ci.sh",
"watch": "NODE_OPTIONS=--max_old_space_size=32384 ts-node ./ci/dev/watch.ts"
"watch": "VSCODE_IPC_HOOK_CLI= NODE_OPTIONS=--max_old_space_size=32384 ts-node ./ci/dev/watch.ts"
Copy link
Contributor

Choose a reason for hiding this comment

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

ez

Copy link
Member Author

Choose a reason for hiding this comment

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

pz

@code-asher code-asher merged commit ce8577b into master Aug 27, 2020
@code-asher code-asher deleted the open-in branch August 27, 2020 20:04
@shayne
Copy link
Contributor

shayne commented Aug 27, 2020

What do you think about making another bin in package.json called code that hooks into entry.ts? We can work on expanding the functionality to support all the possible flags that the standard code command provides. This code could get its own usage/help as well.

@code-asher
Copy link
Member Author

I think that may be a good idea to provide an extra layer of compatibility. We want to add a VSCODE_OPTIONS environment variable that will accepting anything and pass it as-is down to VS Code's CLI parser so we don't have to end up adding all the flags manually to code-server (#1528).

So with that in mind we could directly import VS Code's CLI parser when invoking code then run our entry.ts after setting VSCODE_OPTIONS (or we could send it via IPC). That way the flags and help would always be up to date without any extra work on our part.

@nhooyr nhooyr mentioned this pull request Aug 29, 2020
Closed
@nhooyr
Copy link
Contributor

nhooyr commented Aug 29, 2020

Lets discuss in this a new issue. Opened #2023

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

@nhooyr nhooyr nhooyr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@code-asher @shayne @nhooyr