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

Show breakpoints in overview ruler #82787

Merged
merged 3 commits into from Oct 18, 2019

Conversation

@anirudhrb
Copy link
Contributor

anirudhrb commented Oct 17, 2019

This makes life easier when working with breakpoints in large files. Introduce a
new setting editor.showBreakpointsInOverviewRuler that controls whether this
is enabled or not. This setting is false by default. Use maroon for breakpoint
markers in the overview ruler to differentiate them from error markers.

Closes #15104.

This makes life easier when working with breakpoints in large files. Introduce a
new setting `editor.showBreakpointsInOverviewRuler` that controls whether this
is enabled or not. This setting is `false` by default. Use maroon for breakpoint
markers in the overview ruler to differentiate them from error markers.

Closes #15104.
@anirudhrb

This comment has been minimized.

Copy link
Contributor Author

anirudhrb commented Oct 17, 2019

This is what it looks like:

Screenshot_20191017_220805

@isidorn isidorn self-assigned this Oct 17, 2019
@isidorn isidorn added the debug label Oct 17, 2019
@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 17, 2019

Great PR, thanks a lot!
Everything is great, except can you move the setting to the debug section. Not the editor section.
The setting should be called
debug.showBreakpointsInOverviewRuler and it should be defined somewhere here

Once you tackle this I would be happy merging in the PR

`debug.showBreakpointsInOverviewRuler`.
@anirudhrb

This comment has been minimized.

Copy link
Contributor Author

anirudhrb commented Oct 18, 2019

@isidorn Moved the setting to the debug section now. Please have a look.

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 18, 2019

@anirudhrb this i great work. Thanks a lot!
I tested it and it works nicely. There are just two more things:

  1. We decided to not expose and make the color configurable. Reason being this should always be aligned with the breakpoint color. So this will be tackled in the future when we make the breakpoint color customisable. So can you please remove the contribution of a new color
  2. BreakpointEditorContribution should update all breakpoint decorations on configuration change of debug.showBreakpointsInOverviewRuler . That way when the user changes this setting he will imediatly see the change and he will not need to close / reopen the edditor

Can you please tackle this and I will merge it in. THanks!

@isidorn isidorn added this to the October 2019 milestone Oct 18, 2019
1. Added a configuration change listener in breakpointEditorContribution
   that resets the decorations if debug.showBreakpointsInOverviewRuler
   changes.
2. Removed the color registry contribution for the breakpoint marker
   since it is tied to the breakpoint color which is not configurable
   today.
@anirudhrb

This comment has been minimized.

Copy link
Contributor Author

anirudhrb commented Oct 18, 2019

@isidorn Thanks for the feedback. I have implemented your suggestions. I am now able to see the effect of the setting change without re-opening the editor.

Please have a look and let me know if something can be done in a better way.

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 18, 2019

Looks great, thanks a lot for this PR
☀️ 🌈

@isidorn isidorn merged commit 0431149 into microsoft:master Oct 18, 2019
2 checks passed
2 checks passed
VS Code #20191018.45 succeeded
Details
license/cla All CLA requirements met.
Details
@anirudhrb anirudhrb deleted the anirudhrb:breakpoints_in_scrollbar branch Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.