Debugger: fix adding a breakpoint on an empty line of code and reclicking after addition #17923

HaudinFlorence · 2025-09-25T07:03:00Z

When clicking on an empty line of code, a breakpoint is added to the first non empty previous line. In case there is already a breakpoint at this line, a second breakpoint was temporarily added in a second gutter line, in the time lapse of the click. To get rid of this behavior and in order to not remove the already present breakpoint, a modification of the _onGutterClick method was proposed.

Previously
Screencast From 2025-09-25 08-57-52.webm

With the solution of this PR (kernel blinking indicates the clicks on the gutter without any addition of a second breakpoint)
Screencast From 2025-09-25 08-59-32.webm

References

Should fix issue #17903

Code changes

Update the _onGutterClick method from EditorHandler class.
Introduce a private _getEffectiveClickedLine method that:

finds the first previous non empty line (the effective clicked line) if the initially clicked line is empty and return isLineEmpty set to true
returns directly the clicked line if the line is non empty and a isLineEmpty set to false

In the __onGutterClick, when clicking at the effective clicked position:

if there is no breakpoint, always add one
if there is already a breakpoint at the effective clicked position:
- remove it if the line is not empty
- make the already present breakpoint the selected to inform the user of what happens if the initially clicked position is not empty

User-facing changes

Backwards-incompatible changes

…e the case of trying to add a breakpoint on an empty line of code.

jupyterlab-probot · 2025-09-25T07:03:03Z

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

krassowski · 2025-09-25T07:42:08Z

Maybe if we are redirecting to a different line which already has a breakpoint that breakpoint should become selected to give user a feedback on what just happened?

JohanMabille · 2025-09-25T07:59:51Z

packages/debugger/src/handlers/editor.ts

+              targetLine.number
+            )
+          );
+        }


This code is more or less a duplicate from this one.

I think it can be avoided by first handling the case clickedLine.text.trim() == '' to compute the new position, and then pass it to the part that updates the breakpoints.

Thanks for the review @JohanMabille. The comment has been taken into account in the following commit.

…od and update _onGutterClick method accordingly.

HaudinFlorence · 2025-09-25T11:23:44Z

Maybe if we are redirecting to a different line which already has a breakpoint that breakpoint should become selected to give user a feedback on what just happened?

@krassowski Thanks for your suggestion. I missed your comment. I will try to implement this now.

JohanMabille

The current changes look good to me.

…clicking on an empty line.

HaudinFlorence · 2025-09-26T10:51:08Z

Maybe if we are redirecting to a different line which already has a breakpoint that breakpoint should become selected to give user a feedback on what just happened?

This is new behavior, taking your comment into account @krassowski
Screencast From 2025-09-26 12-49-51.webm

martinRenou · 2025-09-26T11:01:00Z

Maybe if we are redirecting to a different line which already has a breakpoint that breakpoint should become selected to give user a feedback on what just happened?

This is new behavior, taking your comment into account @krassowski Screencast From 2025-09-26 12-49-51.webm

Looks great!!

…int assignment when removing the selected breakpoint.

HaudinFlorence · 2025-09-26T11:04:43Z

I just make a slight change, in order to not have the selected breakpoint becoming red when clicking for its removal
Screencast From 2025-09-26 13-02-26.webm

HaudinFlorence · 2025-09-26T12:50:03Z

UI tests are failing but it doesn't seem related to this PR.

krassowski · 2025-09-26T13:36:06Z

UI tests are failing but it doesn't seem related to this PR.

Most likely unrelated but we would want a green status before merging - if you could review #17928 that would help to fix it!

krassowski · 2025-09-26T14:33:08Z

packages/debugger/src/handlers/editor.ts

+        }
+      }
+    } else {
+      (isLineEmpty = false), (targetLine = clickedLine);


This is rather unusual style. Is this intentional?

No it is not intentional. Will change it to a more usual style.

krassowski · 2025-09-26T14:34:14Z

Notice:   1 failed
    [jupyterlab] › test/jupyterlab/notebook-mermaid-diagrams.test.ts:107:11 › Notebook Mermaid Diagrams dark › Mermaid Diagram 18 architecture in dark theme

This would be unrelated.

krassowski

Thank you @HaudinFlorence!

HaudinFlorence · 2025-09-27T15:44:36Z

Thank you @HaudinFlorence!

Thank you for the review and for the help @krassowski

Update _onGutterClick method from the EditorHandler to properly handl…

d6391a5

…e the case of trying to add a breakpoint on an empty line of code.

github-actions bot added the pkg:debugger label Sep 25, 2025

github-actions bot assigned HaudinFlorence Sep 25, 2025

HaudinFlorence added the bug label Sep 25, 2025

pr-triage-board bot added this to PR triage (experimental) Sep 25, 2025

HaudinFlorence requested review from krassowski and martinRenou September 25, 2025 07:28

JohanMabille reviewed Sep 25, 2025

View reviewed changes

Take review comment into account. Add a _getEffectiveClickedLine meth…

925e931

…od and update _onGutterClick method accordingly.

HaudinFlorence requested a review from JohanMabille September 25, 2025 10:48

krassowski added this to the 4.5.0 milestone Sep 25, 2025

HaudinFlorence changed the title ~~Debugger: fix behavior when trying to add a breakpoint on an empty line of code with the first previous non empty line already has a breakpoint~~ Debugger: fix adding a breakpoint on an empty line of code and reclicking after addition Sep 25, 2025

JohanMabille reviewed Sep 25, 2025

View reviewed changes

HaudinFlorence added 2 commits September 26, 2025 12:38

Apply selected breakpoint style on the previous line breakpoint when …

ed9c2f3

…clicking on an empty line.

Remove a remaining console.log.

16b0d94

Remove unnecessary _debuggerService.model.breakpoints.selectedBreakpo…

43c4ba1

…int assignment when removing the selected breakpoint.

HaudinFlorence closed this Sep 26, 2025

github-project-automation bot moved this to Done in PR triage (experimental) Sep 26, 2025

HaudinFlorence reopened this Sep 26, 2025

Merge branch 'main' into fix_breakpoint_on_empty_line_behavior

e34878f

krassowski reviewed Sep 26, 2025

View reviewed changes

Update _getEffectiveClickedLine method in editorHandler class.

5caa757

HaudinFlorence force-pushed the fix_breakpoint_on_empty_line_behavior branch from 0a1d300 to 5caa757 Compare September 26, 2025 14:52

krassowski approved these changes Sep 26, 2025

View reviewed changes

krassowski merged commit 1945b54 into jupyterlab:main Sep 26, 2025
83 checks passed

pr-triage-board bot removed this from PR triage (experimental) Sep 26, 2025

HaudinFlorence mentioned this pull request Sep 27, 2025

Debugger: adding a breakpoint on an empty line of code and trying to remove it #17903

Closed

Uh oh!

Debugger: fix adding a breakpoint on an empty line of code and reclicking after addition #17923

Debugger: fix adding a breakpoint on an empty line of code and reclicking after addition #17923

Uh oh!

Conversation

HaudinFlorence commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References

Code changes

User-facing changes

Backwards-incompatible changes

Uh oh!

jupyterlab-probot bot commented Sep 25, 2025

Uh oh!

krassowski commented Sep 25, 2025

Uh oh!

JohanMabille Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaudinFlorence Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

HaudinFlorence commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohanMabille left a comment

Choose a reason for hiding this comment

Uh oh!

HaudinFlorence commented Sep 26, 2025

Uh oh!

martinRenou commented Sep 26, 2025

Uh oh!

HaudinFlorence commented Sep 26, 2025

Uh oh!

HaudinFlorence commented Sep 26, 2025

Uh oh!

krassowski commented Sep 26, 2025

Uh oh!

krassowski Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

HaudinFlorence Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

krassowski commented Sep 26, 2025

Uh oh!

krassowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HaudinFlorence commented Sep 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HaudinFlorence commented Sep 25, 2025 •

edited

Loading

JohanMabille Sep 25, 2025 •

edited

Loading

HaudinFlorence commented Sep 25, 2025 •

edited

Loading