Skip to content

Fixes #94 - adds support for conditional breakpoints #172

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

Conversation

rkeithhill
Copy link
Contributor

This adds some unit tests for conditional breakpoints and refactors some existing unit tests I wrote to be a bit more DRY. Also, VSCode supports column breakpoints in its protocol. And PowerShell supports breakpoints on columns other than 1. So why don't I see any UI in VSCode to create a breakpoint at a specific column? Is there something we have to do to declare support for column breakpoints?

This adds some unit tests for conditional breakpoints and refactors some existing unit tests I wrote to be a bit more DRY.  Also, VSCode supports column breakpoints in its protocol.  And PowerShell supports breakpoints on columns other than 1.  So why don't I see any UI in VSCode to create a breakpoint at a specific column?  Is there something we have to do to declare support for column breakpoints?
Assert.Equal(10, breakpoints[1].LineNumber);
Assert.False(breakpoints[1].Verified);
Assert.NotNull(breakpoints[1].Message);
Assert.Contains("Unexpected token '-ez'", breakpoints[1].Message);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome!

@daviwil
Copy link
Contributor

daviwil commented Feb 28, 2016

This is great work man, I'm really excited for this feature to come online! If you're ready you can merge it.

@daviwil
Copy link
Contributor

daviwil commented Feb 28, 2016

Regarding column breakpoints, I haven't seen anything in the UI for that yet. Was sort of expecting that I could put the cursor somewhere, right click and Add Breakpoint but it's not there. We might have to ask about that.

@rkeithhill
Copy link
Contributor Author

It is weird that the protocol supports column breakpoints but the UI doesn't. Oh well. At least we will have conditional breakpoints.

BTW I ran into a "gotcha" - ScriptBlock parsing will catch a number of parse errors but it won't catch this $i == 3. It parses as an assignment with $i on the LHS and, get this, = 3 on the RHS. So yeah, in PowerShell you can actually create a command named = e.g.:

function = { Write-Warning "What do you think this is - C#?" }
$i == 1

Outputs:

WARNING: What do you think this is - C#?

This is a bummer because somebody will invariably forget to use -eq and the breakpoint won't hit. I guess I could try to handle two "special" cases e.g.LHS == RHS and LHS > RHS by inspecting the AST and generating an error message? The other combinations of !=, >=, < and <= generate parse errors. What do you think?

@daviwil
Copy link
Contributor

daviwil commented Feb 28, 2016

Yeah, if there's a way to catch that and throw an error (especially suggesting the usage of -eq and -gt) then that would be ideal. This is a weird case where I'd expect the language would throw an error since it's kind of a common issue for people coming over from C#. I know I've hit it in the past...

…e breakpoint conditon message like $i == 3 or $i > 5.
@rkeithhill
Copy link
Contributor Author

Ask and you shall receive:
vscodebadcondmessage1
vscodebadcondmessage2

rkeithhill added a commit that referenced this pull request Feb 28, 2016
…akpoints

Fixes #94 - adds support for conditional breakpoints
@rkeithhill rkeithhill merged commit 639aba2 into PowerShell:master Feb 28, 2016
@rkeithhill rkeithhill deleted the rkeithhill/is94-impl-cond-breakpoints branch February 28, 2016 21:44
@daviwil
Copy link
Contributor

daviwil commented Feb 29, 2016

That looks great!

TylerLeonhardt pushed a commit to TylerLeonhardt/PowerShellEditorServices that referenced this pull request Feb 26, 2019
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