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

Avoid recording debugger commands in the history #1704

Merged
merged 1 commit into from
Feb 16, 2022

Conversation

andyleejordan
Copy link
Member

This changes how we decide to run a given command through
ExecuteInDebugger or ExecuteNormally. It now restricts the former
such that it is only used for PowerShell's intrinsic debugger commands
(like s, c, etc.) and the latter is used for any other user command
or, more importantly, our own implementation's debugger commands (like
when we call Get-Variable).

@andyleejordan
Copy link
Member Author

The debugger commands haven't changed in eons (at least not since Sergei and I imported Source Depot to GitHub. I'd love to reference them directly but this super helpful class is marked internal (which is also why I can't use the ProcessCommand which accepts an InvocationSettings).

@andyleejordan
Copy link
Member Author

Uh oh things are broken.

@andyleejordan
Copy link
Member Author

Interesting...

[xUnit.net 00:00:16.40]     DebuggerSetsVariablesNoConversion [FAIL]
  Failed DebuggerSetsVariablesNoConversion [910 ms]
  Error Message:
   Assert.Equal() Failure
          ↓ (pos 0)
Expected: Goodbye
Actual:   "Goodbye"
          ↑ (pos 0)

I actually wonder if this fixes a regression in the tests when we did the pipeline rewrite.

@andyleejordan andyleejordan force-pushed the andschwa/debugger-commands branch 2 times, most recently from 1200f62 to 1e8804a Compare February 11, 2022 21:33
VariableScope localScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.LocalScopeName);
// TODO: Fix this so it has the second quotes again?
Copy link
Member Author

Choose a reason for hiding this comment

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

The quotes are back!

@andyleejordan andyleejordan marked this pull request as ready for review February 14, 2022 18:26
@andyleejordan
Copy link
Member Author

Need to test with PowerShell 5.1.

@andyleejordan andyleejordan force-pushed the andschwa/debugger-commands branch from 1e8804a to 3cad765 Compare February 16, 2022 19:07
This changes how we decide to run a given command through
`ExecuteInDebugger` or `ExecuteNormally`. It now restricts the former
such that it is only used for PowerShell's intrinsic debugger commands
(like `s`, `c`, etc.) and the latter is used for any other user command
or, more importantly, our own implementation's debugger commands (like
when we call `Get-Variable`).
@andyleejordan andyleejordan force-pushed the andschwa/debugger-commands branch from 3cad765 to a759b74 Compare February 16, 2022 19:52
@andyleejordan
Copy link
Member Author

Ok @PaulHigin yes, it did need to be made case-insensitive. Fixed that. And I tested manually with Windows PowerShell 5.1, all still works as expected!

@andyleejordan andyleejordan enabled auto-merge (squash) February 16, 2022 19:53
@andyleejordan andyleejordan merged commit 54d548e into master Feb 16, 2022
@andyleejordan andyleejordan deleted the andschwa/debugger-commands branch February 16, 2022 20:05
@andyleejordan
Copy link
Member Author

andyleejordan commented Apr 12, 2022

@SydneyhSmith SydneyhSmith mentioned this pull request Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants