Skip to content
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.

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

feat(starter): provide an executable to start forge in a vscode debugger compatible way #198

Merged
merged 2 commits into from Apr 17, 2017

Conversation

MarshallOfSound
Copy link
Member

@MarshallOfSound MarshallOfSound commented Apr 16, 2017

Currently only on Darwin and Linux, win32 implementation to come

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

Basically adds a new binary to target with VS Code that mangles the args so we can pass node debug args through node without them making the forge host process enter debugging mode. 😆

…ger compatible way

Currently only on darwin and linux, win32 implemenation to come
@Hum4n01d
Copy link

Yay! Looks like the checks are failing though…

Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

That is a fairly interesting hack to get VSCode to work.

Perhaps add a test so that the code coverage doesn't go down?

README.md Outdated
easy with Forge. Simply add this as a launch config in VSCode and you're
good to go.

```json
Copy link
Member

Choose a reason for hiding this comment

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

Might want to change this to javascript since comments aren't a thing 😄

Choose a reason for hiding this comment

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

Done in my version

@@ -47,6 +48,13 @@ import { start } from './api';
runAsNode: !!program.runAsNode,
};

if (program.vscode && appArgs) {
appArgs = appArgs
// Args are in the format ~arg~ so we need to strip the "~"
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I'd put the comment one line above and not split up the statement.

Choose a reason for hiding this comment

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

Done

@malept
Copy link
Member

malept commented Apr 17, 2017

Looks like the checks are failing though…

One of them is because of #110.

@Hum4n01d
Copy link

@malept Hmmm ok, when will this be merged?

@malept
Copy link
Member

malept commented Apr 17, 2017

Once the requested changes are addressed? 😄

@Hum4n01d
Copy link

Is there a way for me to make the changes?

@malept
Copy link
Member

malept commented Apr 17, 2017

You could make a pull request on this pull request branch (instead of master).

Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

This can be merged assuming the tests pass (modulo the intermittent failure).

@malept malept merged commit 9cb7f42 into master Apr 17, 2017
@malept malept deleted the vscode-debug branch April 17, 2017 03:22
@Hum4n01d
Copy link

NPM release?

@malept
Copy link
Member

malept commented Apr 17, 2017

I should have a minute before your comment. Did it not publish correctly?

@Hum4n01d
Copy link

Sorry, it’s working now 😉

dsanders11 pushed a commit that referenced this pull request Jan 14, 2023
feat: Update all dependencies (Electron 5)
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.

None yet

3 participants