From ff187a10f68b54f9d17f2ecbcc5a4a24d3e3e55d Mon Sep 17 00:00:00 2001 From: Lee Dong Geon <100822592+ldg030201@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:13:50 +0900 Subject: [PATCH] fix(debugger): put no breakpoint on blank or comment-only lines (#1678) A gutter click, Toggle Line Breakpoint or Toggle Temporary Line Breakpoint on a blank or comment-only line added a breakpoint the debugger can never stop at. Those lines now take no new breakpoint; comments come from the syntax tree, so this works for every language with a parser, and only blank lines are skipped otherwise. An existing breakpoint on such a line can still be removed. Fixes #1676 Co-authored-by: ldg030201 <100822592+ldg030201@users.noreply.github.com> --- .../debugger/__tests__/actions.test.ts | 17 +++++++++- .../debugger/__tests__/codeLines.test.ts | 30 +++++++++++++++++ .../debugger/__tests__/editor.test.ts | 11 +++++++ src/features/debugger/actions.ts | 11 +++++-- .../debugger/breakpoints/codeLines.ts | 33 +++++++++++++++++++ .../debugger/editor/breakpointGutter.ts | 9 +++-- 6 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 src/features/debugger/__tests__/codeLines.test.ts create mode 100644 src/features/debugger/breakpoints/codeLines.ts diff --git a/src/features/debugger/__tests__/actions.test.ts b/src/features/debugger/__tests__/actions.test.ts index 0e89449d..c0ec8622 100644 --- a/src/features/debugger/__tests__/actions.test.ts +++ b/src/features/debugger/__tests__/actions.test.ts @@ -53,7 +53,7 @@ import { executeAction, getAction } from "@/platform/actions"; import { intellijMacos } from "@/platform/keymaps/intellij-macos"; import type { DebugCapabilities } from "@/platform/ipc"; import { registerDebugActions } from "../actions"; -import { breakpointsOf, loadBreakpoints } from "../breakpoints/store"; +import { breakpointsOf, loadBreakpoints, toggleBreakpoint } from "../breakpoints/store"; import { loadSpecialBreakpoints, removeAllSpecial, useSpecialBreakpoints } from "../breakpoints/special"; import { publishContext, updateExecutionPoint, useExecutionPoint, useDebug, type DebugSession } from "../state"; import { focusDebugView, blurDebugView } from "../ui/focusedView"; @@ -253,6 +253,21 @@ describe("debugger actions (IDEA parity)", () => { expect(opened.at(-1)).toEqual({ path: "/p/app.py", focus: true }); }); + it("puts no new breakpoint on a blank line", async () => { + for (const action of ["debug.toggleBreakpoint", "debug.toggleTemporaryBreakpoint"]) { + for (const line of [1, 2]) { + caret("/p/a.py", "python", "x = 1\n\n \ny = 2", line); + await executeAction(action); + } + expect(breakpointsOf("/p/a.py"), action).toHaveLength(0); + } + // One placed before the line went blank can still be toggled off. + toggleBreakpoint("/p/a.py", 1); + caret("/p/a.py", "python", "x = 1\n\n \ny = 2", 1); + await executeAction("debug.toggleBreakpoint"); + expect(breakpointsOf("/p/a.py")).toHaveLength(0); + }); + it("toggles temporary, method and field breakpoints at the caret", async () => { caret("/p/a.py", "python", "x = 1\ndef run():\n pass", 0); await executeAction("debug.toggleTemporaryBreakpoint"); diff --git a/src/features/debugger/__tests__/codeLines.test.ts b/src/features/debugger/__tests__/codeLines.test.ts new file mode 100644 index 00000000..3f26209a --- /dev/null +++ b/src/features/debugger/__tests__/codeLines.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { java } from "@codemirror/lang-java"; +import { lineHasCode } from "../breakpoints/codeLines"; + +const lines = (state: EditorState) => Array.from({ length: state.doc.lines }, (_, i) => lineHasCode(state, i)); + +describe("lines that take a breakpoint", () => { + it("skips blank and comment-only lines", () => { + const doc = [ + "class A {", + "", + " \t", + " // note", + " /** doc", + " * more", + " */", + " int f() { return 1; } // trailing", + " /* a */ int x = 1;", + " /* a */ /* b */", + "}", + ].join("\n"); + const state = EditorState.create({ doc, extensions: [java()] }); + expect(lines(state)).toEqual([true, false, false, false, false, false, false, true, true, false, true]); + }); + + it("only skips blank lines in a file without a parser", () => { + expect(lines(EditorState.create({ doc: "x\n\n \n// y" }))).toEqual([true, false, false, true]); + }); +}); diff --git a/src/features/debugger/__tests__/editor.test.ts b/src/features/debugger/__tests__/editor.test.ts index 626bc4ee..dd42f354 100644 --- a/src/features/debugger/__tests__/editor.test.ts +++ b/src/features/debugger/__tests__/editor.test.ts @@ -153,6 +153,17 @@ describe("breakpoint gutter clicks", () => { expect(breakpointsOf(ctx.path)[0]).toMatchObject({ line: 0, temporary: true }); }); + it("adds nothing on a blank line but still removes a breakpoint there", () => { + localStorage.clear(); + loadBreakpoints("/proj"); + const view = makeView("\na"); + for (const init of [{}, { altKey: true }, { shiftKey: true }]) click(view, init); + expect(breakpointsOf(ctx.path)).toHaveLength(0); + toggleBreakpoint(ctx.path, 0); + click(view, {}); + expect(breakpointsOf(ctx.path)).toHaveLength(0); + }); + it("middle-click enables / disables an existing breakpoint", () => { localStorage.clear(); loadBreakpoints("/proj"); diff --git a/src/features/debugger/actions.ts b/src/features/debugger/actions.ts index 944dda9f..5772dc0a 100644 --- a/src/features/debugger/actions.ts +++ b/src/features/debugger/actions.ts @@ -7,6 +7,7 @@ import { activeEditor } from "@/editor/api"; import type { EditorView } from "@codemirror/view"; import { maybeEditorService, notifications, showStatusInfo, toolWindowService } from "@/workbench/contributions"; import { classFileBreakpointReason } from "@/platform/classFiles"; +import { lineHasCode } from "./breakpoints/codeLines"; import { canPlaceBreakpoint } from "./breakpoints/languages"; import { removeAllBreakpoints, @@ -628,6 +629,9 @@ export function registerDebugActions(): () => void { return; } const ed = activeEditor(); + // Like IDEA, no new breakpoint on a blank or comment-only line; an existing one still goes. + const taken = exists || specialAt(c.path).some((b) => b.line === c.line); + if (!taken && ed && !lineHasCode(ed.view.state, c.line)) return; if (ed) toggleBreakpointSmart(c.path, c.line, language, ed.view.state.doc.toString()); else toggleBreakpoint(c.path, c.line); }, @@ -640,8 +644,11 @@ export function registerDebugActions(): () => void { description: "A line breakpoint that is removed after it is hit", run: () => { const c = cursor(); - if (c && breakpointPathAllowed(c.path) && canPlaceBreakpoint(activeEditor()?.ctx.language ?? "", c.path)) - toggleTemporaryBreakpoint(c.path, c.line); + const ed = activeEditor(); + if (!c || !breakpointPathAllowed(c.path) || !canPlaceBreakpoint(ed?.ctx.language ?? "", c.path)) return; + const exists = useBreakpoints.getState().byFile[c.path]?.some((b) => b.line === c.line); + if (!exists && ed && !lineHasCode(ed.view.state, c.line)) return; + toggleTemporaryBreakpoint(c.path, c.line); }, }, { diff --git a/src/features/debugger/breakpoints/codeLines.ts b/src/features/debugger/breakpoints/codeLines.ts new file mode 100644 index 00000000..05544033 --- /dev/null +++ b/src/features/debugger/breakpoints/codeLines.ts @@ -0,0 +1,33 @@ +/** + * IDEA puts a new line breakpoint only where there is code: a gutter click or Toggle Line + * Breakpoint on a blank or comment-only line does nothing. Comments come from the syntax + * tree; in a file without a parser only blank lines are skipped. + */ +import { ensureSyntaxTree, syntaxTree } from "@codemirror/language"; +import type { EditorState } from "@codemirror/state"; +import type { SyntaxNode, Tree } from "@lezer/common"; + +const PARSE_BUDGET_MS = 20; + +function commentAt(tree: Tree, pos: number): SyntaxNode | null { + for (let n: SyntaxNode | null = tree.resolveInner(pos, 1); n; n = n.parent) if (/comment/i.test(n.name)) return n; + return null; +} + +/** Whether `line` (0-based) has anything besides whitespace and comments. */ +export function lineHasCode(state: EditorState, line: number): boolean { + if (line < 0 || line >= state.doc.lines) return false; + const { from, to, text } = state.doc.line(line + 1); + if (!/\S/.test(text)) return false; + const tree = ensureSyntaxTree(state, to, PARSE_BUDGET_MS) ?? syntaxTree(state); + for (let pos = from; pos < to;) { + if (/\s/.test(text[pos - from]!)) { + pos++; + continue; + } + const comment = commentAt(tree, pos); + if (!comment) return true; + pos = Math.max(pos + 1, comment.to); + } + return false; +} diff --git a/src/features/debugger/editor/breakpointGutter.ts b/src/features/debugger/editor/breakpointGutter.ts index a81be943..cb744750 100644 --- a/src/features/debugger/editor/breakpointGutter.ts +++ b/src/features/debugger/editor/breakpointGutter.ts @@ -18,6 +18,7 @@ import { import { classFileBreakpointReason } from "@/platform/classFiles"; import { tx } from "@/platform/i18n"; import { showStatusInfo } from "@/workbench/contributions"; +import { lineHasCode } from "../breakpoints/codeLines"; import { canPlaceBreakpoint } from "../breakpoints/languages"; import { removeSpecial, @@ -232,9 +233,10 @@ export function breakpointGutter(ctx: EditorDocContext): Extension { } /** - * IDEA's gutter clicks: click toggles; on an empty line ⇧-click adds a non-suspending - * logging breakpoint and opens its properties, ⌥-click a temporary one. On an existing - * breakpoint only a plain (or ⌘/Ctrl) click removes it; middle-click enables/disables. + * IDEA's gutter clicks: click toggles; on a line without a breakpoint ⇧-click adds a + * non-suspending logging breakpoint and opens its properties, ⌥-click a temporary one. + * Blank and comment-only lines take none. On an existing breakpoint only a plain + * (or ⌘/Ctrl) click removes it; middle-click enables/disables. */ onMouseDown = (e: MouseEvent) => { if ((e.button !== 0 && e.button !== 1) || !inBreakpointArea(e)) return; @@ -271,6 +273,7 @@ export function breakpointGutter(ctx: EditorDocContext): Extension { else removeBreakpoint(path, bp!.id); return; } + if (!lineHasCode(this.view.state, line)) return; if (e.altKey) { addBreakpoint(path, line, { temporary: true }); return;