feat(console): show line numbers next to console output#1008
feat(console): show line numbers next to console output#1008iahmedgamal wants to merge 20 commits into
Conversation
- supports JS, TS and React - remaining languages shows nothing for now - add VLQ decoder and source line map builder (utils/source-map.ts) - addN badge into Luna Console log items via insert event
✅ Deploy Preview for livecodes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis PR adds source-map generation to React and TypeScript compilers, extends compile and console contracts, maps runtime locations to markup or script lines, and displays mapped line numbers in console entries with unit and end-to-end coverage. ChangesConsole Source Map Line Numbers
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Compiler
participant ResultFrame
participant ConsolePanel
participant DOM
Compiler->>ConsolePanel: setSourceMap(sourceMap)
ResultFrame->>ResultFrame: derive console call-site line and source
ResultFrame->>ConsolePanel: postMessage(lineNumber, source, console args)
ConsolePanel->>ConsolePanel: map generated line to original line
ConsolePanel->>DOM: render console-line-number badge
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/livecodes/utils/source-map.ts (1)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
/*@pure*/annotations to exported pure utility functions.Both
decodeVlqandbuildSourceLineMapare exported pure functions insrc/livecodes/utils/but lack the required/*@PURE*/annotation for tree-shaking.As per coding guidelines: "Mark exported pure utility functions with
/*@PURE*/annotation for tree-shaking" (src/livecodes/utils/**/*.{ts,tsx}).♻️ Proposed fix
-export const decodeVlq = (str: string, pos: number): [value: number, nextPos: number] => { +export const decodeVlq = /* `@__PURE__` */ (str: string, pos: number): [value: number, nextPos: number] => {-export const buildSourceLineMap = (sourceMapStr: string): Map<number, number> => { +export const buildSourceLineMap = /* `@__PURE__` */ (sourceMapStr: string): Map<number, number> => {Also applies to: 30-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/livecodes/utils/source-map.ts` at line 3, The exported pure utility functions in source-map.ts need `/* `@__PURE__` */` annotations for tree-shaking. Add the annotation to both `decodeVlq` and `buildSourceLineMap` at their export declarations so bundlers can recognize them as pure. Keep the change limited to these exported utility function definitions in `src/livecodes/utils/source-map.ts`.Source: Coding guidelines
src/livecodes/styles/app.scss (1)
1247-1259: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBadge may not sit at the far right without an auto start margin.
flex-shrink: 0/align-self: centerimply the log item is a flex row, but nothing pushes the badge to the right edge, so it will render immediately after the log content rather than the "far right" shown in the PR screenshots. If the intent is right-alignment, add an auto inline-start margin.🎨 Optional: push badge to the far right
.console-line-number { flex-shrink: 0; align-self: center; + margin-inline-start: auto; padding-right: 8px; padding-left: 4px;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/livecodes/styles/app.scss` around lines 1247 - 1259, The .luna-console-log-item badge styling in console-line-number is missing the flex spacing needed to pin it to the far right. Update the existing .console-line-number rule to use an auto inline-start margin so the badge is pushed away from the log content and aligns to the row’s end while preserving the current flex behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/livecodes/languages/typescript/lang-typescript.ts`:
- Line 51: The TypeScript language setup is dropping the `errors` returned by
`ts.convertCompilerOptionsFromJson`, which can hide invalid or conflicting
compiler settings and leave `sourceMapText` unset. Update `getLanguageInfo` in
`lang-typescript.ts` to capture the full result from
`convertCompilerOptionsFromJson`, then surface any conversion errors into
`info.errors` alongside existing diagnostics so callers can see the problem
instead of silently losing line-number mapping.
In `@src/livecodes/result/utils.ts`:
- Around line 120-125: The getCallSiteLine helper currently depends on
stack.split('\n')[3], which is V8-specific and can break on other engines.
Update getCallSiteLine to parse the Error.stack more defensively by normalizing
stack frames or using frame markers before extracting the caller line, and keep
the fallback behavior returning undefined when no valid call site can be found.
---
Nitpick comments:
In `@src/livecodes/styles/app.scss`:
- Around line 1247-1259: The .luna-console-log-item badge styling in
console-line-number is missing the flex spacing needed to pin it to the far
right. Update the existing .console-line-number rule to use an auto inline-start
margin so the badge is pushed away from the log content and aligns to the row’s
end while preserving the current flex behavior.
In `@src/livecodes/utils/source-map.ts`:
- Line 3: The exported pure utility functions in source-map.ts need `/*
`@__PURE__` */` annotations for tree-shaking. Add the annotation to both
`decodeVlq` and `buildSourceLineMap` at their export declarations so bundlers
can recognize them as pure. Keep the change limited to these exported utility
function definitions in `src/livecodes/utils/source-map.ts`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5b13f2b7-970c-4497-a6df-9ef1c2270431
📒 Files selected for processing (9)
src/livecodes/core.tssrc/livecodes/languages/react/lang-react.tssrc/livecodes/languages/typescript/lang-typescript.tssrc/livecodes/models.tssrc/livecodes/result/utils.tssrc/livecodes/styles/app.scsssrc/livecodes/toolspane/console.tssrc/livecodes/utils/__tests__/source-map.spec.tssrc/livecodes/utils/source-map.ts
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — minor robustness suggestions inline.
Reviewed changes — add source-mapped console line numbers for JS/TS/React, with a custom VLQ decoder/source-line map builder and queue-based badge injection into Luna Console log items.
- Pass
sourceMapfrom the compiled script result to the console viasetSourceMap. - Capture the call-site line in the sandboxed result iframe and forward it with each console message.
- Build a generated-line → original-line map from source map
mappings, keeping only the first segment per generated line. - Render
:Nline-number badges in Luna Console log items through itsinsertevent. - Add unit tests for the custom VLQ decoder and source-line map builder.
ℹ️ Nitpicks
- Four changed files currently fail
prettier --check:src/livecodes/core.ts,src/livecodes/languages/react/lang-react.ts,src/livecodes/result/utils.ts, andsrc/livecodes/utils/__tests__/source-map.spec.ts. Runningnpm run fix:prettierwill clean them up.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
|
Oh wow! I confirm it works with js, ts & react I will review the changes carefully and come back to you, isA. Meanwhile, here are some comments:
You may also want to check comments by coderabbit and pullfrog AI reviews. Thank you :) |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues in the incremental changes — the
fix: eslintcommit addresses the prior feedback it targeted. One minor annotation nit inline.
Reviewed changes — the fix: eslint commit resolves lint/format issues and follows up on prior review comments.
- Fixed TypeScript compiler option error handling in
src/livecodes/languages/typescript/lang-typescript.ts—convertCompilerOptionsFromJsonerrors are now captured and logged as warnings. - Added
/* @__PURE__ */annotation tobuildSourceLineMapinsrc/livecodes/utils/source-map.ts. - Resolved prettier/eslint formatting in
src/livecodes/core.ts,src/livecodes/languages/react/lang-react.ts,src/livecodes/utils/__tests__/source-map.spec.ts, and related files.
A prior robustness suggestion about defensive stack-frame parsing in src/livecodes/result/utils.ts (open thread) remains unchanged by these commits.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
example: (script:7 , or markup:5), consecutive identical logs from the same source are grouped by Luna's native dedeuplication but with badge (script:1:5) - add e2e console-line-logs spec with 5 tests
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/livecodes/toolspane/console.ts (1)
88-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: address static-analysis nits.
SonarCloud flags
String.prototype.match(Line 88) in favor ofRegExp.exec, andparseInt(Lines 91, 92, 112) in favor ofNumber.parseInt.♻️ Suggested tweaks
- const match = current.match(/^(\w+):(\d+)(?::(\d+))?$/); + const match = /^(\w+):(\d+)(?::(\d+))?$/.exec(current); if (!match) return current; const src = match[1]; - const firstLine = parseInt(match[2], 10); - const lastLine = match[3] ? parseInt(match[3], 10) : firstLine; + const firstLine = Number.parseInt(match[2], 10); + const lastLine = match[3] ? Number.parseInt(match[3], 10) : firstLine;- const newLine = parseInt(sourceLine.split(':')[1], 10); + const newLine = Number.parseInt(sourceLine.split(':')[1], 10);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/livecodes/toolspane/console.ts` around lines 88 - 112, Address the static-analysis nits in the line-number handling helper and setupInsertListener: replace String.prototype.match with the equivalent regular expression exec call, and replace every parseInt invocation with Number.parseInt while preserving the existing radix and behavior.Source: Linters/SAST tools
src/livecodes/result/markup-script-lines.ts (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared helpers to avoid duplication.
toPositiveLineNumberandtoUserLineare identically defined in bothmarkup-script-lines.tsandconsole-line-source.ts. Extract them to a shared module (e.g.,result/line-utils.ts) and import from both files to prevent divergence.♻️ Proposed shared module
// src/livecodes/result/line-utils.ts export const toPositiveLineNumber = (line: number | undefined): number | undefined => { if (typeof line !== 'number' || !Number.isFinite(line) || line <= 0) return undefined; return Math.trunc(line); }; export const toUserLine = (docLine: number, offset: number): number | undefined => { if (!offset) return undefined; return toPositiveLineNumber(docLine - offset); };Then in both
markup-script-lines.tsandconsole-line-source.ts:-const toPositiveLineNumber = (line: number | undefined): number | undefined => { - if (typeof line !== 'number' || !Number.isFinite(line) || line <= 0) return undefined; - return Math.trunc(line); -}; - -const toUserLine = (docLine: number, offset: number): number | undefined => { - if (!offset) return undefined; - return toPositiveLineNumber(docLine - offset); -}; +import { toPositiveLineNumber, toUserLine } from './line-utils';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/livecodes/result/markup-script-lines.ts` around lines 1 - 9, Extract the duplicated toPositiveLineNumber and toUserLine helpers from markup-script-lines.ts and console-line-source.ts into a shared result/line-utils.ts module, exporting both functions. Remove the local definitions and import the shared helpers in both consuming files, preserving their existing behavior.e2e/specs/console-line-logs.spec.ts (1)
138-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest 5 could wait for the expected count directly.
waitForConsoleEntries(app, 1)waits for only 1 entry, then relies onwaitForTimeout(300)for the second entry to appear. Since bothconsole.log("same text")calls fire synchronously (inline markup script + editor script), waiting for 2 entries directly would be more robust and eliminate the fragile timeout.♻️ Proposed fix
- await waitForConsoleEntries(app, 1); - await app.waitForTimeout(300); + await waitForConsoleEntries(app, 2); + await app.waitForTimeout(300);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/specs/console-line-logs.spec.ts` around lines 138 - 154, Update the test “does not merge same text from markup and script” to call waitForConsoleEntries(app, 2) and remove the subsequent fixed waitForTimeout(300), so it waits directly for both synchronous console entries before collecting them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/livecodes/result/console-line-source.ts`:
- Around line 99-126: Update proxyConsole’s window.error handler to pass the
ErrorEvent through getConsoleErrorSite instead of posting error.lineno directly.
Use the returned lineNumber, source, rawLine, and offset metadata when
constructing the posted runtime error payload so window errors receive the same
markup/script mapping as console calls.
In `@src/livecodes/toolspane/console.ts`:
- Around line 168-198: Prevent lineNumberQueue from advancing for console
methods that do not create a rendered log item, including console.time,
console.countReset, and passing console.assert calls. Update the
message-processing logic around lineNumberQueue and the insert handler so badge
metadata is queued or assigned only when Luna actually inserts a console entry,
preserving alignment for subsequent source badges.
---
Nitpick comments:
In `@e2e/specs/console-line-logs.spec.ts`:
- Around line 138-154: Update the test “does not merge same text from markup and
script” to call waitForConsoleEntries(app, 2) and remove the subsequent fixed
waitForTimeout(300), so it waits directly for both synchronous console entries
before collecting them.
In `@src/livecodes/result/markup-script-lines.ts`:
- Around line 1-9: Extract the duplicated toPositiveLineNumber and toUserLine
helpers from markup-script-lines.ts and console-line-source.ts into a shared
result/line-utils.ts module, exporting both functions. Remove the local
definitions and import the shared helpers in both consuming files, preserving
their existing behavior.
In `@src/livecodes/toolspane/console.ts`:
- Around line 88-112: Address the static-analysis nits in the line-number
handling helper and setupInsertListener: replace String.prototype.match with the
equivalent regular expression exec call, and replace every parseInt invocation
with Number.parseInt while preserving the existing radix and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 90eec2b2-d314-4db5-be15-fe4409be1c7f
📒 Files selected for processing (8)
e2e/specs/console-line-logs.spec.tssrc/livecodes/result/console-line-source.tssrc/livecodes/result/markup-script-lines.tssrc/livecodes/result/result-page.tssrc/livecodes/result/result-types.tssrc/livecodes/result/utils.tssrc/livecodes/toolspane/console.tssrc/livecodes/utils/__tests__/source-map.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/livecodes/utils/tests/source-map.spec.ts
…nsole code to use it
There was a problem hiding this comment.
Important
isExternalScriptFrame fails to classify external scripts whose URLs contain query strings or hashes, which can produce wrong source labels and line numbers for logs from those scripts.
Reviewed changes — the latest commit replaces the hardcoded stack-frame index with a defensive source-aware classifier, distinguishes markup vs. script sources, supports grouped line-number badges for deduplicated logs, and adds e2e coverage.
- Replaced brittle stack-frame parsing in
src/livecodes/result/utils.tsand the newsrc/livecodes/result/console-line-source.tswith frame filtering, external-script preference, and safe fallbacks. - Added markup vs. script source detection so inline
<script>logs and editor-script logs display the correct source label. - Added grouped line ranges for Luna Console's native deduplication (e.g.
script:1:5). - Added e2e tests in
e2e/specs/console-line-logs.spec.tscovering script logs, inline markup logs, regrouping after interrupts, and mixed-source separation.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/livecodes/utils/line-number.ts`:
- Around line 1-4: Add the `/* `@__PURE__` */` annotation to the exported
`toPositiveLineNumber` utility declaration so bundlers can tree-shake this
side-effect-free function, preserving its existing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aa62eea9-5558-4028-b38a-623056da2fb7
📒 Files selected for processing (5)
src/livecodes/result/console-line-source.tssrc/livecodes/result/markup-script-lines.tssrc/livecodes/toolspane/console.tssrc/livecodes/utils/__tests__/line-number.spec.tssrc/livecodes/utils/line-number.ts
✅ Files skipped from review due to trivial changes (1)
- src/livecodes/utils/tests/line-number.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/livecodes/result/console-line-source.ts
- src/livecodes/result/markup-script-lines.ts
- src/livecodes/toolspane/console.ts
There was a problem hiding this comment.
ℹ️ No new Pullfrog issues in this incremental commit. The prior
toPositiveLineNumberduplication concern is now resolved.
Reviewed changes — the latest commit extracts toPositiveLineNumber into a shared utility and adds unit tests for it.
- Extracted shared
toPositiveLineNumberutility intosrc/livecodes/utils/line-number.ts. - Removed duplicated definitions from
src/livecodes/result/console-line-source.ts,src/livecodes/result/markup-script-lines.ts, andsrc/livecodes/toolspane/console.ts, replacing them with imports. - Added unit tests in
src/livecodes/utils/__tests__/line-number.spec.tscovering valid inputs and edge cases.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
I kept the Luna native grouping way, but added the new line badge to keep the grouping and 
|
|
Thank you @iahmedgamal I had a good look. Here are some comments (bear with me 😅):
So, I prefer to disable grouping messages in console unless they are from the same source and line (e.g. repeated invocations of same function or logs in a loop, etc). And then we can avoid using ranges.
I found some bugs:
console.log('hi');
console.time('timer');
console.log('hello');
Here are some feature requests 😄 :
<!doctype html>
<html lang="en">
<script>
const message = "Hello from TypeScript!";
const logIt = (msg) => {
console.log(msg);
};
logIt(message);
//# sourceMappingURL=data:application/json;charset=utf-8,%7B%22version%22%3A3%2C%22sources%22%3A%5B%22app.ts%22%5D%2C%22mappings%22%3A%22%3BAAAA%3BAACA%3BAACA%3BAACA%3BAACA%22%2C%22sourcesContent%22%3A%5B%22const%20message%3A%20string%20%3D%20'Hello%20from%20TypeScript!'%3B%5Cnconst%20logIt%20%3D%20(msg%3A%20string)%3A%20void%20%3D%3E%20%7B%5Cn%20%20%20%20console.log(msg)%3B%5Cn%7D%3B%5CnlogIt(message)%3B%22%5D%7D
</script>
</body>
</html>I could not get that to work in LiveCodes. I'm not sure if this is related to restriction on sandboxed iframe, or because we use document.write to add content to the page or something else. Any way, this is not a high priority, but a really nice-to-have feature. Some organizational comments:
I do not want to overload you if you think this scope is larger than what you want to contribute with. Thank you. I really appreciate your work. |
- Moved all code in console-line-source.ts, markup-script-lines.ts, result-types.ts, line-number.ts, source-map.ts to a single file in compiler/source-maps.ts && Create tests for source map
- config.tools?.enabled as string[])?.includes('console') to skip invocation with every markup that has no script
- removed the range line numbers line logs
- handled the time, countReset, assert, table to expect no visual on the line
- navigate to the line when clicking on the log
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes — the latest commit refactors source-map handling into a single module, switches console line mapping to a Record keyed by filename, fixes queue alignment for silent console methods, and adds e2e coverage for source maps, queue reset, and named files.
- Consolidated source-map console utilities in
src/livecodes/compiler/source-maps.tsby merging VLQ decoding, line-number helpers, markup inline-script resolution, and call-site detection into one module. - Switched source map contract to
Record<string, string>soconfig.scriptFilenamecan drive the console badge label (e.g.tax-calculator.ts:5) instead of a genericscriptkey. - Fixed console badge queue alignment for silent methods (
console.time,console.countReset, passingassert, emptytable) so they no longer consume a queue slot. - Made line-number badges clickable to navigate to the corresponding editor line.
- Expanded e2e coverage for TypeScript source-map mapping, named files, queue reset, loop grouping, and silent-method queue behavior.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Hey @hatemhosny thanks for the review and hte kind words, appericate it, -I addressed all the points,
Thanks for your time and the guidance for easier testing please take a look (looks amazing lol ) screen-capture.5.webm |
|
I LIKE IT 📣📣📣📣📣 Thank you @iahmedgamal By the way, you can test it online on the preview URL: https://deploy-preview-1008--livecodes.netlify.app/ |
- add markup inline script e2e
…gamal/livecodes into pr/iahmedgamal/1008
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — the latest commit addresses the two outstanding Pullfrog comments and adds regression coverage for markup inline scripts.
- Fixed React source-map key in
src/livecodes/languages/react/lang-react.tsso it usesconfig.scriptFilename ?? 'script', matching the TypeScript path. - Corrected the console source-map comment in
src/livecodes/toolspane/console.tsto remove the inaccuratemessage.filenamemulti-file hint. - Extended the e2e helper in
e2e/specs/console-line-logs.spec.tsto support arbitrary language query params (e.g.react). - Added regression tests for TypeScript markup inline-script line numbers and for same-value logs staying separate across JavaScript, TypeScript, and React.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
|
Thank you @iahmedgamal I have added some changes. This is a brief summary:
Please review the changes and let me know what you think. By the way, I found a bug: some text
<script>
console.log('hello') // correct (markup:4)
throw new Error('error') // incorrect (script:5)
</script> |
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes — the latest commit adds runtime-error line mapping for the result iframe, refactors the call-site resolver into a shared helper, and adds regression coverage for markup inline-script runtime errors.
- Added
getConsoleCallSiteFromErrorinsrc/livecodes/compiler/source-maps.tssowindow.onerrorcan recover source and original line numbers. - Wired the result-iframe
errorevent through the new mapper, replacing rawerror.lineno. - Added
//# sourceURL=livecodes-script.jsto the single-file editor script so editor-script stack frames classify as external and source maps apply correctly. - Added Playwright coverage for markup inline-script runtime-error line numbers.
ℹ️ Nitpicks
e2e/specs/console-line-logs.spec.tscurrently failsprettier --check(mixed single/double quotes and some over-long lines). Runningnpm run fix:prettierwill clean it up.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
| }; | ||
| }; | ||
|
|
||
| export const getConsoleCallSiteFromError = ( |
There was a problem hiding this comment.
| export const getConsoleCallSiteFromError = ( | |
| export const getConsoleCallSiteFromError = /* @__PURE__ */ (lineNumber: unknown, stack?: string): ConsoleCallSite => { |
…d other non-JS compilers.
I saw the changes and tested it locally, works as expected,
|
There was a problem hiding this comment.
✅ No new issues found in this increment.
Reviewed changes — the latest commit makes //# sourceURL conditional on JS-compatible script types so non-JS interpreters are no longer broken by an injected JavaScript comment.
- Restricted
//# sourceURL=livecodes-script.jsto JS-compatible scripts insrc/livecodes/result/result-page.tsby resolving the script type first (languageScriptType,customSettings.scriptType, orisModuleScript) and only appending the sourceURL suffix whenscriptTypeis missing,'module', or matches/(java|ecma)script/i. - Preserved existing JS/TS/React behavior while fixing Python, Lua, PHP, SQL, and other non-JS languages whose compilers use a custom
script.type.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
|
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes — the latest commit relaxes exact markup line-number assertions to bounded ranges and ordering checks in the console-line-logs e2e spec.
- Reduced e2e flakiness in
e2e/specs/console-line-logs.spec.tsby replacing exactmarkup:4/markup:7assertions with ranges (log line 3–5, error line 6–8) and enforcing ordering for same-value inline-script logs.
Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
| const sameValueEntries = entries.filter((e) => e.text === 'same value'); | ||
|
|
||
| // Assert | ||
| expect(sameValueEntries, `${testCase.label}: should show two separate same-value entries`).toHaveLength(2); |
There was a problem hiding this comment.
This file still fails prettier --check after the latest change (npx prettier --check e2e/specs/console-line-logs.spec.ts fails). The new assertion lines added here are over the print width and mix quote styles with the rest of the file. Running npm run fix:prettier will clean them up.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/livecodes/toolspane/console.ts (1)
182-219: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMap line numbers for every non-silent rendered method.
The allowlist omits badges for
dir,table,count,timeLog,timeEnd, failing assertions, and groups, despite those calls producing Luna entries and carrying source metadata.Proposed fix
- const lineDisplayMethods = ['log', 'error', 'warn', 'info', 'output']; if ( lineNumbersEnabled && message.lineNumber !== undefined && - lineDisplayMethods.includes(message.method) + !message.silent ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/livecodes/toolspane/console.ts` around lines 182 - 219, Update the console message handling around lineDisplayMethods so every non-silent method that produces a Luna DOM entry—including dir, table, count, timeLog, timeEnd, failing assertions, and groups—uses the same source and line-number mapping path. Preserve the existing queue alignment behavior for silent methods, and ensure each rendered entry receives its mapped badge and deduplication handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/specs/console-line-logs.spec.ts`:
- Around line 338-369: Replace the permissive range assertions in the markup
runtime-error test with exact source-line assertions: require logLine to equal
markup:4 and errLine to equal markup:7. In e2e/specs/console-line-logs.spec.ts
lines 371-439, update every language-case assertion to require markup:9 and
markup:10 respectively, removing the numeric parsing and range checks where no
longer needed.
- Around line 500-522: Update the rerun portion of the test “badge queue resets
correctly after page re-run” to wait for a distinct second-run console entry
rather than relying on the existing first-run entry. Ensure the assertion after
waitForResultUpdate verifies the newly produced “hello” entry has sourceLine
“script:2”, so broken queue handling cannot pass using stale data.
In `@src/livecodes/compiler/source-maps.ts`:
- Around line 68-76: Update the source-map parsing logic around the mappings
loop in src/livecodes/compiler/source-maps.ts lines 68-76 to retain
generated-column information and use the stack-frame column when selecting the
applicable segment instead of keeping only the first segment per line. In
src/livecodes/compiler/__tests__/source-maps.spec.ts lines 202-214, replace the
“first segment wins” expectation with assertions verifying column-specific
source resolution.
- Around line 60-69: Validate that the parsed mappings value is a string before
calling split in the source-map parsing flow. Update the JSON parsing logic
around the mappings variable to return the existing lineMap fallback when
mappings is missing or not a string, while preserving normal processing for
valid string mappings.
- Around line 122-126: Update getMarkupInlineScriptLine so the docLine <
markupOffset fallback does not always use getMarkupInlineScriptStartLines()[0].
Preserve the current inline script identity in the metadata and use its
corresponding start line, or skip this remapping when multiple inline scripts
exist and identity is unavailable; retain the existing single-script behavior.
In `@src/livecodes/editor/codemirror/codemirror.ts`:
- Around line 499-502: Restore 1-based EditorPosition handling in setPosition:
clamp lineNumber to the valid 1..view.state.doc.lines range, clamp the column to
the line’s valid 1-based bounds, and retain the conversion needed for
CodeMirror’s zero-based position API. In src/livecodes/editor/monaco/monaco.ts
lines 605-612, change the fallback column used by the Monaco position flow from
0 to 1; both sites must preserve symmetry with getPosition().
---
Outside diff comments:
In `@src/livecodes/toolspane/console.ts`:
- Around line 182-219: Update the console message handling around
lineDisplayMethods so every non-silent method that produces a Luna DOM
entry—including dir, table, count, timeLog, timeEnd, failing assertions, and
groups—uses the same source and line-number mapping path. Preserve the existing
queue alignment behavior for silent methods, and ensure each rendered entry
receives its mapped badge and deduplication handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c0aef0a0-130b-4a28-a567-e3fc3860efff
📒 Files selected for processing (15)
e2e/specs/console-line-logs.spec.tssrc/livecodes/compiler/__tests__/source-maps.spec.tssrc/livecodes/compiler/source-maps.tssrc/livecodes/core.tssrc/livecodes/editor/codemirror/codemirror.tssrc/livecodes/editor/monaco/monaco.tssrc/livecodes/events/custom-events.tssrc/livecodes/languages/react/lang-react.tssrc/livecodes/languages/typescript/lang-typescript.tssrc/livecodes/models.tssrc/livecodes/result/result-page.tssrc/livecodes/result/utils.tssrc/livecodes/styles/app.scsssrc/livecodes/toolspane/console.tssrc/sdk/internal.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/livecodes/languages/react/lang-react.ts
- src/livecodes/languages/typescript/lang-typescript.ts
| test('markup runtime errors should map to markup line numbers', async ({ page, getTestUrl }) => { | ||
| const { app } = await openWithCode(page, getTestUrl, { | ||
| markup: [ | ||
| 'some text', | ||
| '', | ||
| '<script>', | ||
| ' console.log("line-number-bug-log")', | ||
| '', | ||
| '', | ||
| ' throw new Error("line-number-bug-error")', | ||
| '</script>', | ||
| ].join('\n'), | ||
| script: '', | ||
| }); | ||
| await waitForConsoleEntries(app, 2); | ||
| await app.waitForTimeout(300); | ||
|
|
||
| const entries = await getConsoleEntries(app); | ||
| const logLine = entries.find((e) => e.text.includes('line-number-bug-log'))?.sourceLine; | ||
| const errLine = entries.find((e) => e.text.includes('line-number-bug-error'))?.sourceLine; | ||
|
|
||
| expect(logLine).toMatch(/^markup:\d+$/); | ||
| expect(errLine).toMatch(/^markup:\d+$/); | ||
|
|
||
| const logNum = Number(logLine?.split(':')[1]); | ||
| const errNum = Number(errLine?.split(':')[1]); | ||
|
|
||
| expect(logNum).toBeGreaterThanOrEqual(3); | ||
| expect(logNum).toBeLessThanOrEqual(5); | ||
| expect(errNum).toBeGreaterThanOrEqual(6); | ||
| expect(errNum).toBeLessThanOrEqual(8); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use exact assertions for deterministic markup source lines. Permissive ranges allow the central off-by-one regression to escape coverage.
e2e/specs/console-line-logs.spec.ts#L338-L369: assertmarkup:4andmarkup:7.e2e/specs/console-line-logs.spec.ts#L371-L439: assertmarkup:9andmarkup:10for every language case.
📍 Affects 1 file
e2e/specs/console-line-logs.spec.ts#L338-L369(this comment)e2e/specs/console-line-logs.spec.ts#L371-L439
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/specs/console-line-logs.spec.ts` around lines 338 - 369, Replace the
permissive range assertions in the markup runtime-error test with exact
source-line assertions: require logLine to equal markup:4 and errLine to equal
markup:7. In e2e/specs/console-line-logs.spec.ts lines 371-439, update every
language-case assertion to require markup:9 and markup:10 respectively, removing
the numeric parsing and range checks where no longer needed.
| test('badge queue resets correctly after page re-run', async ({ page, getTestUrl }) => { | ||
| // Arrange — console.time is silent; after re-run the queue must start fresh. | ||
| // Scenario: run with time between two logs → 'hello' gets script:2. | ||
| // Then re-run (same code) → queue resets, 'hello' still gets script:2, not null. | ||
| const { app, waitForResultUpdate } = await openWithCode(page, getTestUrl, { | ||
| markup: '', | ||
| script: ["console.time('t');", "console.log('hello');"].join('\n'), | ||
| }); | ||
| await waitForConsoleEntries(app, 1); | ||
|
|
||
| // First run: 'hello' should get badge script:2 | ||
| let entries = await getConsoleEntries(app); | ||
| expect(entries.find((e) => e.text === 'hello')?.sourceLine).toBe('script:2'); | ||
|
|
||
| // Re-run the result page (triggers setSourceMap which now resets the queue) | ||
| await waitForResultUpdate(); | ||
| await waitForConsoleEntries(app, 1); | ||
| await app.waitForTimeout(300); | ||
|
|
||
| // After reload: queue should be reset, 'hello' still gets script:2 (not null) | ||
| entries = await getConsoleEntries(app); | ||
| expect(entries.find((e) => e.text === 'hello')?.sourceLine).toBe('script:2'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait for and assert the second-run entry.
Line 516 only waits for one entry, which already exists from the first run. Line 521 can therefore inspect that stale entry and pass even if rerun queue handling is broken.
Proposed fix
await waitForResultUpdate();
- await waitForConsoleEntries(app, 1);
+ await waitForConsoleEntries(app, 2);
await app.waitForTimeout(300);
- entries = await getConsoleEntries(app);
- expect(entries.find((e) => e.text === 'hello')?.sourceLine).toBe('script:2');
+ const helloEntries = (await getConsoleEntries(app)).filter((e) => e.text === 'hello');
+ expect(helloEntries).toHaveLength(2);
+ expect(helloEntries[1].sourceLine).toBe('script:2');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('badge queue resets correctly after page re-run', async ({ page, getTestUrl }) => { | |
| // Arrange — console.time is silent; after re-run the queue must start fresh. | |
| // Scenario: run with time between two logs → 'hello' gets script:2. | |
| // Then re-run (same code) → queue resets, 'hello' still gets script:2, not null. | |
| const { app, waitForResultUpdate } = await openWithCode(page, getTestUrl, { | |
| markup: '', | |
| script: ["console.time('t');", "console.log('hello');"].join('\n'), | |
| }); | |
| await waitForConsoleEntries(app, 1); | |
| // First run: 'hello' should get badge script:2 | |
| let entries = await getConsoleEntries(app); | |
| expect(entries.find((e) => e.text === 'hello')?.sourceLine).toBe('script:2'); | |
| // Re-run the result page (triggers setSourceMap which now resets the queue) | |
| await waitForResultUpdate(); | |
| await waitForConsoleEntries(app, 1); | |
| await app.waitForTimeout(300); | |
| // After reload: queue should be reset, 'hello' still gets script:2 (not null) | |
| entries = await getConsoleEntries(app); | |
| expect(entries.find((e) => e.text === 'hello')?.sourceLine).toBe('script:2'); | |
| }); | |
| test('badge queue resets correctly after page re-run', async ({ page, getTestUrl }) => { | |
| // Arrange — console.time is silent; after re-run the queue must start fresh. | |
| // Scenario: run with time between two logs → 'hello' gets script:2. | |
| // Then re-run (same code) → queue resets, 'hello' still gets script:2, not null. | |
| const { app, waitForResultUpdate } = await openWithCode(page, getTestUrl, { | |
| markup: '', | |
| script: ["console.time('t');", "console.log('hello');"].join('\n'), | |
| }); | |
| await waitForConsoleEntries(app, 1); | |
| // First run: 'hello' should get badge script:2 | |
| let entries = await getConsoleEntries(app); | |
| expect(entries.find((e) => e.text === 'hello')?.sourceLine).toBe('script:2'); | |
| // Re-run the result page (triggers setSourceMap which now resets the queue) | |
| await waitForResultUpdate(); | |
| await waitForConsoleEntries(app, 2); | |
| await app.waitForTimeout(300); | |
| // After reload: queue should be reset, 'hello' still gets script:2 (not null) | |
| const helloEntries = (await getConsoleEntries(app)).filter((e) => e.text === 'hello'); | |
| expect(helloEntries).toHaveLength(2); | |
| expect(helloEntries[1].sourceLine).toBe('script:2'); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/specs/console-line-logs.spec.ts` around lines 500 - 522, Update the rerun
portion of the test “badge queue resets correctly after page re-run” to wait for
a distinct second-run console entry rather than relying on the existing
first-run entry. Ensure the assertion after waitForResultUpdate verifies the
newly produced “hello” entry has sourceLine “script:2”, so broken queue handling
cannot pass using stale data.
| let mappings: string; | ||
| try { | ||
| ({ mappings } = JSON.parse(sourceMapStr) as { mappings: string }); | ||
| } catch { | ||
| return lineMap; // invalid JSON — nothing to map | ||
| } | ||
| if (!mappings) return lineMap; | ||
|
|
||
| let originalLine = 0; | ||
| mappings.split(';').forEach((group, compiledLine) => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the parsed mappings type before calling split.
Valid JSON such as {"mappings":{}} bypasses the catch and throws at Line 69, potentially breaking console rendering.
Proposed fix
- let mappings: string;
+ let parsed: unknown;
try {
- ({ mappings } = JSON.parse(sourceMapStr) as { mappings: string });
+ parsed = JSON.parse(sourceMapStr);
} catch {
return lineMap; // invalid JSON — nothing to map
}
- if (!mappings) return lineMap;
+ if (
+ typeof parsed !== 'object' ||
+ parsed === null ||
+ !('mappings' in parsed) ||
+ typeof parsed.mappings !== 'string' ||
+ !parsed.mappings
+ ) {
+ return lineMap;
+ }
+ const mappings = parsed.mappings;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mappings: string; | |
| try { | |
| ({ mappings } = JSON.parse(sourceMapStr) as { mappings: string }); | |
| } catch { | |
| return lineMap; // invalid JSON — nothing to map | |
| } | |
| if (!mappings) return lineMap; | |
| let originalLine = 0; | |
| mappings.split(';').forEach((group, compiledLine) => { | |
| let parsed: unknown; | |
| try { | |
| parsed = JSON.parse(sourceMapStr); | |
| } catch { | |
| return lineMap; // invalid JSON — nothing to map | |
| } | |
| if ( | |
| typeof parsed !== 'object' || | |
| parsed === null || | |
| !('mappings' in parsed) || | |
| typeof parsed.mappings !== 'string' || | |
| !parsed.mappings | |
| ) { | |
| return lineMap; | |
| } | |
| const mappings = parsed.mappings; | |
| let originalLine = 0; | |
| mappings.split(';').forEach((group, compiledLine) => { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/livecodes/compiler/source-maps.ts` around lines 60 - 69, Validate that
the parsed mappings value is a string before calling split in the source-map
parsing flow. Update the JSON parsing logic around the mappings variable to
return the existing lineMap fallback when mappings is missing or not a string,
while preserving normal processing for valid string mappings.
| let originalLine = 0; | ||
| mappings.split(';').forEach((group, compiledLine) => { | ||
| for (const segment of group.split(',')) { | ||
| const delta = decodeOriginalLineDelta(segment); | ||
| if (delta === null) continue; | ||
| originalLine += delta; // delta is relative to previous segment across all lines | ||
| if (!lineMap.has(compiledLine + 1)) { | ||
| lineMap.set(compiledLine + 1, originalLine + 1); // convert 0-indexed to 1-indexed | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Source-map resolution discards the generated column required to select the correct segment.
src/livecodes/compiler/source-maps.ts#L68-L76: retain generated-column mappings and resolve against the stack-frame column.src/livecodes/compiler/__tests__/source-maps.spec.ts#L202-L214: replace the “first segment wins” expectation with column-specific assertions.
📍 Affects 2 files
src/livecodes/compiler/source-maps.ts#L68-L76(this comment)src/livecodes/compiler/__tests__/source-maps.spec.ts#L202-L214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/livecodes/compiler/source-maps.ts` around lines 68 - 76, Update the
source-map parsing logic around the mappings loop in
src/livecodes/compiler/source-maps.ts lines 68-76 to retain generated-column
information and use the stack-frame column when selecting the applicable segment
instead of keeping only the first segment per line. In
src/livecodes/compiler/__tests__/source-maps.spec.ts lines 202-214, replace the
“first segment wins” expectation with assertions verifying column-specific
source resolution.
| if (docLine < markupOffset) { | ||
| const firstScriptStartLine = getMarkupInlineScriptStartLines()[0]; | ||
| if (firstScriptStartLine) { | ||
| return toPositiveLineNumber(firstScriptStartLine + docLine - 1); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
# Locate the target file and related helpers.
git ls-files 'src/livecodes/compiler/source-maps.ts' 'src/livecodes/compiler/*' | sed -n '1,120p'
echo '--- outline ---'
ast-grep outline src/livecodes/compiler/source-maps.ts --view expanded || true
echo '--- relevant excerpts ---'
cat -n src/livecodes/compiler/source-maps.ts | sed -n '1,260p'Repository: live-codes/livecodes
Length of output: 11871
Avoid rebasing later inline scripts to the first one
When document.currentScript is null, getMarkupInlineScriptLine() falls back to getMarkupInlineScriptStartLines()[0] for docLine < markupOffset. That can point errors from later inline scripts at the wrong source. Preserve per-script identity in the metadata, or skip the remap when multiple inline scripts exist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/livecodes/compiler/source-maps.ts` around lines 122 - 126, Update
getMarkupInlineScriptLine so the docLine < markupOffset fallback does not always
use getMarkupInlineScriptStartLines()[0]. Preserve the current inline script
identity in the metadata and use its corresponding start line, or skip this
remapping when multiple inline scripts exist and identity is unavailable; retain
the existing single-script behavior.
| const setPosition = ({ lineNumber, column = 0 }: EditorPosition) => { | ||
| const line = view.state.doc.lines > lineNumber ? lineNumber : view.state.doc.lines; | ||
| const lineInfo = view.state.doc.line(line); | ||
| const columnNumber = lineInfo.length > col ? col : lineInfo.length; | ||
| const columnNumber = lineInfo.length > column ? column : lineInfo.length; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Restore 1-based column semantics and safe boundaries.
Editor columns are universally 1-based (as seen in browser stack traces, Monaco's API, and CodeMirror's own getPosition() which returns + 1). By defaulting to 0 and removing the - 1 offset conversion in CodeMirror, the PR introduces a 1-character off-by-one bug for real error columns and breaks symmetry between getPosition and setPosition.
Additionally, CodeMirror 6's doc.line(n) strictly requires 1 <= n <= doc.lines and throws a RangeError if it receives a 0, which necessitates robust bounds clamping.
src/livecodes/editor/codemirror/codemirror.ts#L499-L502: Restore the 1-based column calculation and useMath.min/Math.maxto safely clamp both the line and column to valid document bounds.src/livecodes/editor/monaco/monaco.ts#L605-L612: Default the fallback column to1instead of0to align with the 1-basedEditorPositioncontract.
🛠️ Proposed fix for `src/livecodes/editor/codemirror/codemirror.ts`
- const setPosition = ({ lineNumber, column = 0 }: EditorPosition) => {
- const line = view.state.doc.lines > lineNumber ? lineNumber : view.state.doc.lines;
- const lineInfo = view.state.doc.line(line);
- const columnNumber = lineInfo.length > column ? column : lineInfo.length;
+ const setPosition = ({ lineNumber, column }: EditorPosition) => {
+ const validColumn = (column && column > 0) ? column : 1;
+ const line = Math.max(1, Math.min(lineNumber, view.state.doc.lines));
+ const lineInfo = view.state.doc.line(line);
+ const columnNumber = Math.max(0, Math.min(validColumn - 1, lineInfo.length));🛠️ Proposed fix for `src/livecodes/editor/monaco/monaco.ts`
const setPosition = (position: EditorPosition) => {
const newPosition = {
lineNumber: position.lineNumber,
- column: position.column ?? 0,
+ column: (position.column && position.column > 0) ? position.column : 1,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const setPosition = ({ lineNumber, column = 0 }: EditorPosition) => { | |
| const line = view.state.doc.lines > lineNumber ? lineNumber : view.state.doc.lines; | |
| const lineInfo = view.state.doc.line(line); | |
| const columnNumber = lineInfo.length > col ? col : lineInfo.length; | |
| const columnNumber = lineInfo.length > column ? column : lineInfo.length; | |
| const setPosition = ({ lineNumber, column }: EditorPosition) => { | |
| const validColumn = (column && column > 0) ? column : 1; | |
| const line = Math.max(1, Math.min(lineNumber, view.state.doc.lines)); | |
| const lineInfo = view.state.doc.line(line); | |
| const columnNumber = Math.max(0, Math.min(validColumn - 1, lineInfo.length)); |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 502-502: Prefer Math.min() to simplify ternary expressions.
📍 Affects 2 files
src/livecodes/editor/codemirror/codemirror.ts#L499-L502(this comment)src/livecodes/editor/monaco/monaco.ts#L605-L612
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/livecodes/editor/codemirror/codemirror.ts` around lines 499 - 502,
Restore 1-based EditorPosition handling in setPosition: clamp lineNumber to the
valid 1..view.state.doc.lines range, clamp the column to the line’s valid
1-based bounds, and retain the conversion needed for CodeMirror’s zero-based
position API. In src/livecodes/editor/monaco/monaco.ts lines 605-612, change the
fallback column used by the Monaco position flow from 0 to 1; both sites must
preserve symmetry with getPosition().
3 participants
What type of PR is this? (check all applicable)
Description
Console line number is added at the far right of the console section
Related Tickets & Documents
#635
Mobile & Desktop Screenshots/Recordings
Please observe this React template works as well && it's responsive
Added tests?
Added to documentations?
[optional] Are there any post-deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
New Features
Bug Fixes