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

Enable debugging in temporary Integrated Console sessions #883

Merged
merged 2 commits into from
Jun 21, 2017

Conversation

daviwil
Copy link
Contributor

@daviwil daviwil commented Jun 15, 2017

This change enables the user to configure their debugging sessions to
launch a fresh PowerShell process each time they start the debugger.
This is useful when debugging scripts or modules which use PowerShell
classes or managed assemblies which cannot be reloaded within the same
process.

Resolves #367.

@daviwil daviwil added this to the June 2017 milestone Jun 15, 2017
@daviwil daviwil changed the title Enable launching fresh PowerShell instances for debugging WIP: Enable launching fresh PowerShell instances for debugging Jun 15, 2017
@daviwil
Copy link
Contributor Author

daviwil commented Jun 15, 2017

Feedback Needed

I want to add a very clearly named configuration which the user can use to determine whether a new process is created when they start the debugger. I want this setting to be used in 2 places:

  • launch.json configurations
  • User and workspace-level settings

The launch configuration setting in launch.json will always override the workspace-level and user setting.

Configuration name possibilities:

  • reuseConsoleSession
  • createNewSession
  • others?

At the user/workspace setting level we'd prefix the name with powershell.debugging.*.

@rkeithhill
Copy link
Contributor

Do you think this needs to be a setting? Seems like it would just be a debug configuration property e.g. reuseConsoleSession.

We could add one or more configurations that come with this property set both ways. Question is - what would you consider the "normal" way to debug. Personally, I prefer a clean debug session every time. But what would we call debug configurations that persist state between invocations? PowerShell: Debug Current File in Interactive Session?

@kapilmb
Copy link

kapilmb commented Jun 15, 2017

I think reuseConsoleSession sounds more intuitive - it provides some context, in the sense that it refers to an existing entity i.e. the console session.

@adamrushuk
Copy link

+1 for reuseConsoleSession.

@daviwil
Copy link
Contributor Author

daviwil commented Jun 15, 2017

@rkeithhill Having it also be a user setting allows the user to configure the default debugging behavior so that they don't have to set it on each one of their launch.json configurations. I'd like to get to a point where the only time you need to manage a launch.json is when you really want to have multiple (or custom) configs.

Regarding what the default setting is, I'd personally go with a default of not reusing the session, though I'm not sure if that will cause confusion or frustration for former ISE users who expect everything to happen in the same session. I suppose I could prompt the first time they hit F5 without a launch.json and ask what their desired default behavior is?

Having different default launch configs makes sense. We could ship "Debug Current File in Current Session" and "Debug Current File in New Session" configs so that people are aware of the differences.

There's also the possibility of one other setting now that I think about it: endSessionWhenComplete (which really needs a better name). This would cover the scenario where the user wants to run their script and then be dropped back into the command prompt once the script completes so they can poke around with the session. I'd have it be off by default, but good to have it be an option. Might hold off on that one until the following update after getting some feedback. Thoughts?

@daviwil
Copy link
Contributor Author

daviwil commented Jun 16, 2017

Now that I think about it, having a setting name which is intended to be false by default feels a little weird. I added a new commit (6083fe9) that adds a setting called powershell.debugger.newProcessPerSession which should be clearer and indicate that we're creating a new process for each debug session. I've added a matching launch configuration named newProcessPerSession which overrides the user level setting (the name feels a little more clumsy in the launch config).

I'm still not fully pleased with the name. Maybe a boolean is the wrong way to express this? An alternative would be a setting like "defaultSessionType" and a matching launch param of "sessionType" which would take "ExistingSession" or "NewProcess" as possible values. That'd also give us flexibility to add other session types, like if we wanted to allow the user to just create a new runspace in the existing process (but I can't think of a good reason to do that other than launch speed).

@rkeithhill
Copy link
Contributor

having a setting name which is intended to be false by default feels a little weird

But isn't that the nature of "opt-in" settings?

BTW an enum might be OK. In fact, I don't necessarily have to have a new process if I knew I was starting with a "clean" session. That is, if PowerShell had a reset button that wiped all variables created in session, modules loaded in the session, subscribed events, outstanding bg jobs, remote connections, etc. I'd be fine debugging in that as long I was convinced it did a good job cleaning house. :-)

@rkeithhill
Copy link
Contributor

If you do create a new debug session every time does that mean folks loose their language service session each time? If so, that wouldn't be ideal. I would hope the new behavior would be to add another PowerShell Debug Interactive Console to the list of terminals that was there just for that debug session.

@rkeithhill
Copy link
Contributor

rkeithhill commented Jun 16, 2017

Also didn't we have a task to add a workspace specific debugger init script? And for that matter, one also for the language server? This would be much better for projects than to rely upon user-specific profiles.

@daviwil daviwil modified the milestones: June 2017, 1.4.0 Jun 16, 2017
This change moves SessionManager's process launching behavior into a new
class called PowerShellProcess which will be used for launching
additional PowerShell processes to enable fresh, transient debugging
sessions.
@daviwil daviwil force-pushed the debug-only-sessions branch from 6083fe9 to 7648b77 Compare June 21, 2017 14:09
@daviwil daviwil changed the title WIP: Enable launching fresh PowerShell instances for debugging Enable debugging in temporary Integrated Console sessions Jun 21, 2017
This change enables the user to configure their debugging sessions to
launch a temporary Integrated Console each time they start the debugger.
This is useful when debugging scripts or modules which use PowerShell
classes or managed assemblies which cannot be reloaded within the same
process.

Resolves PowerShell#367.
@daviwil daviwil force-pushed the debug-only-sessions branch from 7648b77 to e146f33 Compare June 21, 2017 14:11
@daviwil daviwil merged commit ec53532 into PowerShell:master Jun 21, 2017
@daviwil daviwil deleted the debug-only-sessions branch June 21, 2017 14:14
@ohadschn
Copy link

ohadschn commented Aug 6, 2018

For posterity, at the time of writing the setting's name is powershell.debugging.createTemporaryIntegratedConsole

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.

6 participants