Skip to content

Track tempIntegratedConsole launch param, do not exit when session ends #625

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 1 commit into from
Feb 20, 2018

Conversation

rkeithhill
Copy link
Contributor

@rkeithhill rkeithhill commented Feb 20, 2018

This does not leave a working prompt. I think that is OK for now. This is still better than the current situation.

@@ -303,6 +303,12 @@ private void OnDebugServiceClientConnect(object sender, TcpSocketServerChannel s

this.debugServiceListener.Start();
}
else if (this.debugAdapter.IsUsingTempIntegratedConsole)
Copy link
Member

Choose a reason for hiding this comment

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

So this else if prevents the host from exiting... What's the host in this case?

More simply, what is the user experience change?

Copy link
Contributor Author

@rkeithhill rkeithhill Feb 20, 2018

Choose a reason for hiding this comment

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

The temp debug console does not immediately exit after the script finishes execution or the debug session is stopped. This allows the user to scroll through the window to inspect the script output.

The only downside I've observed at this point is that every temp debug session log file ends with:

2/19/2018 8:48:16 PM [ERROR] - Method "ListenForMessages" at line 344 of C:\Users\Keith\GitHub\rkeithhill\PowerShellEditorServices\src\PowerShellEditorServices.Protocol\MessageProtocol\ProtocolEndpoint.cs

    Stream terminated unexpectedly, ending MessageDispatcher loop
    
    Exception: IOException
    Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host.

This is because VSCode does terminate the process when it starts the next debug session. All things considered, I think this is a fine trade-off.

Copy link
Member

Choose a reason for hiding this comment

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

100% agree!

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM as soon as AppVeyor passes. :)

Copy link
Contributor

@daviwil daviwil left a comment

Choose a reason for hiding this comment

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

Looks good, thanks Keith!

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.

3 participants