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

Add --delay-startup argument to Ark #35

Merged
merged 5 commits into from
Jun 15, 2023
Merged

Add --delay-startup argument to Ark #35

merged 5 commits into from
Jun 15, 2023

Conversation

lionel-
Copy link
Contributor

@lionel- lionel- commented Jun 14, 2023

Progress towards posit-dev/positron#740

Adds a new undocumented argument --startup-delay. It takes the path to a notification file to which the frontend writes when the debugger has been attached. Ark blocks until a change to the file is detected. After that, startup continues as normal.

Copy link
Contributor

@jmcphers jmcphers left a comment

Choose a reason for hiding this comment

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

This LGTM, but I wouldn't have expected an argument called --delay-startup to take a file argument. Maybe if the argument is numeric we could sleep that number of seconds instead, to support more traditional debugger scenarios?

@lionel- lionel- force-pushed the feature/delay-startup branch from a12ca70 to 9f3181e Compare June 15, 2023 10:23
@lionel- lionel- force-pushed the feature/delay-startup branch from 9f3181e to 2ddb6ba Compare June 15, 2023 10:40
@lionel-
Copy link
Contributor Author

lionel- commented Jun 15, 2023

This LGTM, but I wouldn't have expected an argument called --delay-startup to take a file argument.

Good point. I've renamed the argument to --startup-notifier-file.

Maybe if the argument is numeric we could sleep that number of seconds instead, to support more traditional debugger scenarios?

Good idea. I think I'd implement this with a new --startup-delay argument to keep things simple. Otherwise a typo in the supplied argument that causes a parse failure will produce surprising behaviour instead of an error. Now done in last commit.

@lionel- lionel- merged commit 66f7e1d into main Jun 15, 2023
@lionel- lionel- deleted the feature/delay-startup branch June 15, 2023 10:45
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