feat(markup): lineNumbers extension#1112
Open
saxumcordis wants to merge 3 commits into
Open
Conversation
saxumcordis
changed the title
Reviewer's GuideAdds a configurable line-numbers/line-highlighting system for the markup (CodeMirror) editor, wires it through the editor bundle API, styles gutters, exposes helper APIs, and documents the behavior via Storybook demos. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
|
|
|
saxumcordis
force-pushed
the
line_numbers
branch
from
a34984a to
2ea439d
Compare
added 2 commits
April 30, 2026 15:01
saxumcordis
force-pushed
the
line_numbers
branch
from
2ea439d to
28f1cc4
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
MarkdownEditorMarkupConfigyou re-exportMarkupLineNumbersConfigwithout importing it, which will break type checking; add an explicit import from the newmarkup/codemirror/line-highlight/typesmodule (or a barrel) before re-exporting. - The
useEffectinMarkupEditorComponentthat performs scroll-to-line on mount has an empty dependency array but closes overeditorandeditor.markupConfig; either add the proper dependencies or explain via an eslint-disable comment to avoid stale state and hook-lint warnings. - The
initialScrollToLinegetter onEditorImplis currently unused, while the scroll-to-line behavior reads directly fromeditor.markupConfig.lineNumbers; consider either wiring the getter into the React component or removing it to avoid dead API surface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `MarkdownEditorMarkupConfig` you re-export `MarkupLineNumbersConfig` without importing it, which will break type checking; add an explicit import from the new `markup/codemirror/line-highlight/types` module (or a barrel) before re-exporting.
- The `useEffect` in `MarkupEditorComponent` that performs scroll-to-line on mount has an empty dependency array but closes over `editor` and `editor.markupConfig`; either add the proper dependencies or explain via an eslint-disable comment to avoid stale state and hook-lint warnings.
- The `initialScrollToLine` getter on `EditorImpl` is currently unused, while the scroll-to-line behavior reads directly from `editor.markupConfig.lineNumbers`; consider either wiring the getter into the React component or removing it to avoid dead API surface.
## Individual Comments
### Comment 1
<location path="packages/editor/src/markup/codemirror/line-highlight/extension.ts" line_range="51-53" />
<code_context>
+export function lineHighlight(options?: LineHighlightOptions): Extension {
+ const initialRange = options?.initialRange ?? null;
+
+ const highlightedLineField = StateField.define<LineRange | null>({
+ create: () => initialRange,
+ update(value, tr) {
+ for (const effect of tr.effects) {
+ if (effect.is(setHighlightedLine)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Highlighted line range is not adjusted when document content changes, which can desync highlighting from the intended lines.
`highlightedLineField` stores `{from, to}` line indices and only updates when a `setHighlightedLine` effect is present, so edits that insert/remove lines above the range leave it pointing at the wrong lines. Please use transaction data to keep the range in sync with document changes (e.g., map through `tr.changes` or recompute from a stable position), or clearly document that the highlighted range is not preserved across edits.
</issue_to_address>
### Comment 2
<location path="demo/src/stories/examples/markup-line-numbers/Editor.tsx" line_range="114-117" />
<code_context>
+ );
+ const [lastClickedLine, setLastClickedLine] = useState<number | null>(null);
+
+ const markupLineNumbers: MarkupLineNumbersConfig | undefined = lineNumbers
+ ? {
+ ...lineNumbers,
+ onLineClick: (line) => setLastClickedLine(line),
+ }
+ : undefined;
</code_context>
<issue_to_address>
**issue (bug_risk):** The demo overrides `onLineClick`, discarding any user-provided handler.
With this spread + override, any `onLineClick` passed in via `lineNumbers` is replaced, so the caller’s handler never runs. To preserve caller behavior while still updating `lastClickedLine`, compose the handlers instead (e.g., call `lineNumbers.onLineClick?.(line)` before/after `setLastClickedLine`).
</issue_to_address>
### Comment 3
<location path="packages/editor/src/markup/codemirror/line-highlight/extension.ts" line_range="63" />
<code_context>
+ },
+ });
+
+ const highlightGutterDecoration = gutterLineClass.compute([highlightedLineField], (state) => {
+ const range = state.field(highlightedLineField);
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the shared line-to-position logic into a helper function and reusing it to build both decorations without per-iteration try/catch blocks.
You can reduce duplication and avoid `try/catch` in the hot path by introducing a small helper that maps the logical `LineRange` to concrete document positions once, then reuse it for both decorations.
```ts
function highlightedLinePositions(
state: EditorState,
range: LineRange | null,
): number[] {
if (!range) return [];
const docLines = state.doc.lines;
const positions: number[] = [];
for (let line = range.from; line <= range.to; line++) {
if (line < 0 || line >= docLines) continue;
const cmLine = state.doc.line(line + 1);
positions.push(cmLine.from);
}
return positions;
}
```
Use it in `highlightGutterDecoration`:
```ts
const highlightGutterDecoration = gutterLineClass.compute([highlightedLineField], (state) => {
const range = state.field(highlightedLineField);
const positions = highlightedLinePositions(state, range);
if (!positions.length) return RangeSet.empty;
const builder = new RangeSetBuilder<GutterMarker>();
for (const from of positions) {
builder.add(from, from, highlightedGutterClass);
}
return builder.finish();
});
```
And in `highlightLineDecorations`:
```ts
const highlightLineDecorations = EditorView.decorations.compute(
[highlightedLineField],
(state): DecorationSet => {
const range = state.field(highlightedLineField);
const positions = highlightedLinePositions(state, range);
if (!positions.length) return Decoration.none;
const decorations = positions.map((from) =>
highlightLineDecoration.range(from),
);
return Decoration.set(decorations);
},
);
```
This keeps all existing behavior, centralizes the off‑by‑one/bounds logic in one place, and removes the `try/catch` from the tight loops.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
makhnatkin
reviewed
May 18, 2026
Contributor
Author
|
gentle ping |
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
2 participants
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.
Preview
Summary by Sourcery
Add configurable line numbers and line highlighting support to the markup (CodeMirror) editor, and expose the related APIs and demos.
New Features:
Enhancements:
Summary by Sourcery
Add configurable line numbers and line highlighting capabilities to the markup (CodeMirror) editor and surface them through the public editor API and demos.
New Features:
Enhancements:
Documentation: