fix(debugger): keep editor line numbers 1:1 with source for breakpoints#686
Open
andreahlert wants to merge 1 commit into
Open
Conversation
normalizeScript reformatted the displayed script by trimming leading and
trailing blank lines, but breakpoints and the paused-line highlight match
against the tokenizer's original line numbers (token.line). A leading blank
line shifted the display, so gutter clicks and the highlight landed on the
wrong line ("breakpoints sometimes work"). Setting them from code worked,
which matched the report.
- normalizeScript now de-indents horizontally only and never adds or removes
lines, so Monaco display lines stay 1:1 with token.line.
- pauseAt waits for editorReady before highlighting (like showLivePause), so
the paused line is highlighted on the first break instead of being applied
to the not-yet-mounted editor.
Part of bigskysoftware#685
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #685 (item 6).
The bug
normalizeScriptreformats the script for display in the panel editor. It trimmed leading/trailing blank lines, which shifts every line up. But breakpoints and the paused-line highlight match against the tokenizer's original line numbers (token.line). So a script whose_attribute starts with a newline (extremely common in HTML) displayedon clickon Monaco line 1 while the tokenizer has it on line 2. A gutter click stored the Monaco line; the breakpoint check compared the source line; they never matched.That's the "breakpoints sometimes work" report: it worked only when there was no leading blank line. Setting breakpoints from code worked because that path uses source lines directly.
The fix
Make the editor's line numbers stay 1:1 with the source so there is a single coordinate system everywhere (gutter clicks, the breakpoint check, and the paused-line highlight).
normalizeScriptnow de-indents horizontally only and never adds or removes lines. Leading/trailing blank lines are kept as empty lines, so Monaco display line N ==token.lineN. This fixes the gutter mismatch and the paused-line highlight at the root, without scattering offset math across every call site.pauseAtnow waits foreditorReadybefore drawing the current-line decoration (mirroringshowLivePause).selectElementmounts Monaco asynchronously, so the old synchronous highlight was applied to the not-yet-mounted editor and dropped, which is why the paused line frequently showed no highlight on the first break. It now reusesapplyLineDecoration.Verification
normalizeScripton leading-newline, double-leading-newline, and no-leading-newline scripts: display line now equals source line in every case (previously off by the number of leading blank lines), and line count is preserved. No regression on the no-leading-newline case._="\n on click ... breakpoint ..."script: execution pauses at the breakpoint, the Monaco model shows the command on the correct line, and the current line is highlighted (it was not highlighted at all before).Scope
Source-only change to
src/ext/debugger.js.dist/is intentionally left out: the committeddist/is already stale relative to committedsrc/(e.g. the htmx-detection guard), so a rebuild here would pull in unrelated drift. Rebuilddist/separately at release.