Skip to content

Skip serializing our internal language kernel terminal processes on shutdown #217

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

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

petetronic
Copy link
Collaborator

When we quit Positron, we tear down any language runtime kernel processes (which are now managed by the terminal service host).

These internal terminal processes should not be serialized/replayed on restarting Positron.

This change attempts to detect those internal positron kernel terminal processes without changing the terminal service API.

Issue: #213

…hutdown

When we quit Positron, we tear down any language runtime kernel processes (which are now managed by the terminal service host).

These internal terminal processes should not be serialized/replayed on restarting Positron.

This change attempts to detect those internal positron kernel terminal processes without changing the terminal service API.

Issue: #213
@petetronic
Copy link
Collaborator Author

Also added Pty Host attachment to our default debug launch profile for Positron.

@@ -409,7 +409,7 @@ export class JupyterKernel extends EventEmitter implements vscode.Disposable {
const command = args.join(' ');

// Create environment.
const env = {};
const env = { POSITRON_LANGUAGE: this._spec.language };
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Open to any suggestions on what a useful env var might be, for now it's just a flag.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe POSITRON_VERSION, since there's a 95% chance any code (other than ours) that wants to check this value already knows what the language is. But it's all speculative at this point, so this works too!

@petetronic petetronic requested a review from jmcphers March 1, 2023 20:51
Copy link
Collaborator

@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.

LGTM!

@@ -409,7 +409,7 @@ export class JupyterKernel extends EventEmitter implements vscode.Disposable {
const command = args.join(' ');

// Create environment.
const env = {};
const env = { POSITRON_LANGUAGE: this._spec.language };
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe POSITRON_VERSION, since there's a 95% chance any code (other than ours) that wants to check this value already knows what the language is. But it's all speculative at this point, so this works too!

@petetronic petetronic merged commit 9a42b72 into main Mar 1, 2023
@petetronic petetronic deleted the issue-213 branch March 1, 2023 21:50
wesm pushed a commit that referenced this pull request Mar 28, 2024
…python snippets

Merge pull request #217 from posit-dev/feature/1437

Add initial placeholder for python snippets
--------------------
Commit message for posit-dev/positron-python@a2971a6:

Add initial placeholder for python snippets

Establishes a mechanism to address #1437


Authored-by: Pete Farland <[email protected]>
Signed-off-by: Pete Farland <[email protected]>
wesm pushed a commit that referenced this pull request Mar 28, 2024
…python snippets

Merge pull request #217 from posit-dev/feature/1437

Add initial placeholder for python snippets
--------------------
Commit message for posit-dev/positron-python@a2971a6:

Add initial placeholder for python snippets

Establishes a mechanism to address #1437


Authored-by: Pete Farland <[email protected]>
Signed-off-by: Pete Farland <[email protected]>
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