Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

make breakpoints more robust and improve protocol interface #222

Merged
merged 1 commit into from Jun 1, 2016

Conversation

jlongster
Copy link
Contributor

Breakpoints were pretty broken before: sliding breakpoints wouldn't work, and the data returned from each client was actually a bit different. I went ahead and improved the protocol so that "breakpoint clients" are only something the Firefox client has to be concerned with now. Breakpoints should be a lot more robust: sliding works, and adding & removing seems to always work now (previously sometimes I couldn't remove a breakpoint because it got into a bad state).

I also fixed "snippets" with sliding breakpoints: previously it would show "undefined" if the breakpoint moved. Now it properly gets the text from the line that it moved to.

r?

return {
id: bpClient.actor,
actualLocation: actualLocation
};
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want a Breakpoint type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a bad idea, but it's a little more confusing than Source or Location because in most places a breakpoint just is a Location. The UI doesn't have any notion of a breakpointId like does a sourceId, because a Location is everything is needs to talk about breakpoints. At the protocol level we want an id and other things like actualLocation.

We could make a breakpoint type for the protocol though, with id, location, and actualLocation fields. It's clearly for the protocol, given that it has two location fields, which we can use to know if it's moved.

@jasonLaster
Copy link
Contributor

r+

Wow. That was a lot of fixes. Great job

@jlongster
Copy link
Contributor Author

I added breakpoints types, but the actual Breakpoint is not being used yet, it will be used in the reducer. The protocol uses the type BreakpointResult which only contains information to indicate if the breakpoint was moved.

@jlongster jlongster merged commit c125950 into master Jun 1, 2016
@clarkbw clarkbw deleted the better-breakpoints branch August 31, 2016 18:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants