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

feat(debug): highlight current line #5957

Conversation

filipdutescu
Copy link
Contributor

@filipdutescu filipdutescu commented Feb 12, 2023

Add new theme highlight keys, for setting the colour of the breakpoint
character and the current line at which execution has been paused at.
The two new keys are ui.highlight.frameline and ui.debug.breakpoint.
Highlight according to those keys, both the line at which debugging
is paused at and the breakpoint indicator.

Add an indicator for the current line at which execution is paused
at, themed by the ui.debug.active theme scope. Update various themes
to showcase how the new functionality works.

Better icons are dependent on #2869, and as such will be handled in the
future, once it lands.

Closes: #5952
Signed-off-by: Filip Dutescu filip.dutescu@gmail.com

@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 55788ea to 34fe8a9 Compare February 12, 2023 21:35
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

I am gonna test this out tomorrow. Just a couple quick comments.

I think the CI failure is unrelated and a rerun would pass fine

helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
helix-term/src/ui/editor.rs Outdated Show resolved Hide resolved
@filipdutescu
Copy link
Contributor Author

Will push changes that address most of the comments. Still need to resolve the bug in the screenshot and update all themes to use the new highlight keys.

image

@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 34fe8a9 to 3df9d6a Compare February 13, 2023 19:23
@pascalkuthe pascalkuthe added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-debug-adapter Area: Debug adapter client labels Feb 15, 2023
@pascalkuthe
Copy link
Member

I am afraid you misunderstood my hint a bit. All the code you pushed in b26b98e is unnecessary.

Simply add the code below after the following line:
https://github.com/filipdutescu/helix/blob/b26b98ec952d75fe6be2560c503c5b0a014c7af4/helix-term/src/ui/editor.rs#L109

This will

if let Some(frame_line) = current_frame{
         let doc = doc.text();
	 highlights = Box::new(syntax::merge(
	    highlights,
	    vec![(frame_highlight_scope, doc.line_to_char(frame_line), doc.line_to_char(frame_line + 1))],
	));
}

I just wrote this down off the top of my head so there might be a typo somewhere and the code is probably not perfect but it should set you on the right path.
current_frame should be the result of checking whether there is a dap line for the current view (None if there isn't one otherwise it should be Some(line_number)).
frame_highlight_scope should be the highlight scope you obtained from the theme for highlighting the line.

This will remove any foreground/background colors set by syntax highlighting but leave cursors untouched (since those are merged in afterwards)

@filipdutescu
Copy link
Contributor Author

Seems like I overcomplicated the solution a bunch. I did not realise I could just use doc.line_to_char(...), it was exactly what I, somehow, did not see. It will probably similarly easy to change cursor and selection highlights for the dap line.

Thank you for pointing it out!

@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from a9d342f to 9950765 Compare February 19, 2023 21:58
@filipdutescu
Copy link
Contributor Author

Fixed all issues, I just need to also update basically all themes, which I'll do during this week.

@filipdutescu filipdutescu requested review from archseer and pascalkuthe and removed request for pascalkuthe and archseer February 19, 2023 21:59
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 9950765 to c8c3f07 Compare February 20, 2023 21:27
@filipdutescu
Copy link
Contributor Author

filipdutescu commented Feb 22, 2023

Talked a bit more with @pascalkuthe (thanks for being so helpful and patient) and decided to go for a more VSCode design, rather than VS. This simplifies the logic a lot the logic needed for this feature and makes it so we only need to add at minimum 1 new mandatory highlight, ui.debug. I will push the changes tomorrow. Here is an example using my theme:

image

I would still like to allow for more customisation (read as allow VS style frame-lines), but I think it's a lot more important to get the overhaul in an MVP state, rather than focus on customisation and endless architecture discussions. This solution should be a simple one, which should not raise any/many concerns, leading to rapid inclusion.

@filipdutescu
Copy link
Contributor Author

filipdutescu commented Feb 22, 2023

How the default theme will look:

image

@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from c8c3f07 to 26847d4 Compare February 23, 2023 20:30
@filipdutescu filipdutescu requested review from archseer and removed request for pascalkuthe February 23, 2023 20:34
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 26847d4 to bb01335 Compare March 6, 2023 22:03
book/src/themes.md Outdated Show resolved Hide resolved
@filipdutescu
Copy link
Contributor Author

filipdutescu commented Mar 9, 2023

After talking some more with @archseer, we landed on this colour for the debug highlight, which I find beautiful:

image

@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from bb01335 to 33f172d Compare March 9, 2023 18:07
@filipdutescu filipdutescu requested a review from pascalkuthe March 9, 2023 18:08
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 33f172d to 15c1354 Compare March 9, 2023 19:31
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from bda88b1 to 9ec135d Compare March 10, 2023 18:31
pascalkuthe
pascalkuthe previously approved these changes Mar 10, 2023
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this 👍

@pascalkuthe pascalkuthe added this to the next milestone Mar 10, 2023
@filipdutescu
Copy link
Contributor Author

Thank you very much for guiding me through the process! Looking forward to many more contributions 😁

@filipdutescu filipdutescu requested a review from archseer March 10, 2023 18:48
@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2023
return;
}
renderer.surface.set_style(
Rect::new(inner.x, inner.y + pos.visual_line, inner.width, 1),
Copy link
Member

Choose a reason for hiding this comment

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

just a reference for other reviews:

Adding inner,.y is correct here, the fact that it was missing before was a regression from #5420

filipdutescu added a commit to filipdutescu/helix that referenced this pull request Mar 12, 2023
Continuing work done in helix-editor#5957, these changes add column precision
to breakpoints for those set using the keymap command. For those set
with the mouse, preserve the current behaviour, concretely, add a line
breakpoint. This allows for more fine grained control, allowing, for
debuggers which support it, to add breakpoints to scopes such as lambda
functions, callbacks and more.

Change how the stack frame line is highlighted, using a full line for
breakpoints which apply to a line and highlight only the scope for those
that operate only on those scopes.

Closes: helix-editor#6238
Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
filipdutescu added a commit to filipdutescu/helix that referenced this pull request Mar 12, 2023
Continuing work done in helix-editor#5957, these changes add column precision
to breakpoints for those set using the keymap command. For those set
with the mouse, preserve the current behaviour, concretely, add a line
breakpoint. This allows for more fine grained control, allowing, for
debuggers which support it, to add breakpoints to scopes such as lambda
functions, callbacks and more.

Change how the stack frame line is highlighted, using a full line for
breakpoints which apply to a line and highlight only the scope for those
that operate only on those scopes.

Closes: helix-editor#6238
Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
filipdutescu added a commit to filipdutescu/helix that referenced this pull request Mar 12, 2023
Continuing work done in helix-editor#5957, these changes add column precision
to breakpoints for those set using the keymap command. For those set
with the mouse, preserve the current behaviour, concretely, add a line
breakpoint. This allows for more fine grained control, allowing, for
debuggers which support it, to add breakpoints to scopes such as lambda
functions, callbacks and more.

Change how the stack frame line is highlighted, using a full line for
breakpoints which apply to a line and highlight only the scope for those
that operate only on those scopes.

Closes: helix-editor#6238
Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
filipdutescu added a commit to filipdutescu/helix that referenced this pull request Mar 12, 2023
Continuing work done in helix-editor#5957, these changes add column precision
to breakpoints for those set using the keymap command. For those set
with the mouse, preserve the current behaviour, concretely, add a line
breakpoint. This allows for more fine grained control, allowing, for
debuggers which support it, to add breakpoints to scopes such as lambda
functions, callbacks and more.

Change how the stack frame line is highlighted, using a full line for
breakpoints which apply to a line and highlight only the scope for those
that operate only on those scopes.

Closes: helix-editor#6238
Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 9ec135d to a258f6d Compare March 13, 2023 19:57
@filipdutescu
Copy link
Contributor Author

I forgot to set the breakpoint to the proper colour, which I did with this rebase as well:

image

helix-view/src/gutter.rs Outdated Show resolved Hide resolved
helix-view/src/gutter.rs Outdated Show resolved Hide resolved
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from a258f6d to 0d87b48 Compare March 14, 2023 20:30
@filipdutescu filipdutescu requested a review from archseer March 14, 2023 20:30
helix-view/src/gutter.rs Outdated Show resolved Hide resolved
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 0d87b48 to 31be813 Compare March 17, 2023 17:37
@filipdutescu filipdutescu requested a review from archseer March 17, 2023 17:37
Add new theme highlight keys, for setting the colour of the breakpoint
character and the current line at which execution has been paused at.
The two new keys are `ui.highlight.frameline` and `ui.debug.breakpoint`.
Highlight according to those keys, both the line at which debugging
is paused at and the breakpoint indicator.

Add an indicator for the current line at which execution is paused
at, themed by the `ui.debug.active` theme scope. Update various themes
to showcase how the new functionality works.

Better icons are dependent on helix-editor#2869, and as such will be handled in the
future, once it lands.

Closes: helix-editor#5952
Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
@filipdutescu filipdutescu force-pushed the feat/breakpoint_currentline_better_highlights branch from 31be813 to 0dde083 Compare March 25, 2023 09:20
@archseer archseer merged commit d59b805 into helix-editor:master Mar 29, 2023
Triton171 pushed a commit to Triton171/helix that referenced this pull request Jun 18, 2023
Add new theme highlight keys, for setting the colour of the breakpoint
character and the current line at which execution has been paused at.
The two new keys are `ui.highlight.frameline` and `ui.debug.breakpoint`.
Highlight according to those keys, both the line at which debugging
is paused at and the breakpoint indicator.

Add an indicator for the current line at which execution is paused
at, themed by the `ui.debug.active` theme scope. Update various themes
to showcase how the new functionality works.

Better icons are dependent on helix-editor#2869, and as such will be handled in the
future, once it lands.

Closes: helix-editor#5952

Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
wes-adams pushed a commit to wes-adams/helix that referenced this pull request Jul 4, 2023
Add new theme highlight keys, for setting the colour of the breakpoint
character and the current line at which execution has been paused at.
The two new keys are `ui.highlight.frameline` and `ui.debug.breakpoint`.
Highlight according to those keys, both the line at which debugging
is paused at and the breakpoint indicator.

Add an indicator for the current line at which execution is paused
at, themed by the `ui.debug.active` theme scope. Update various themes
to showcase how the new functionality works.

Better icons are dependent on helix-editor#2869, and as such will be handled in the
future, once it lands.

Closes: helix-editor#5952

Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Add new theme highlight keys, for setting the colour of the breakpoint
character and the current line at which execution has been paused at.
The two new keys are `ui.highlight.frameline` and `ui.debug.breakpoint`.
Highlight according to those keys, both the line at which debugging
is paused at and the breakpoint indicator.

Add an indicator for the current line at which execution is paused
at, themed by the `ui.debug.active` theme scope. Update various themes
to showcase how the new functionality works.

Better icons are dependent on helix-editor#2869, and as such will be handled in the
future, once it lands.

Closes: helix-editor#5952

Signed-off-by: Filip Dutescu <filip.dutescu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debug-adapter Area: Debug adapter client S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better highlight breakpoints and current line at which execution is paused at
4 participants