-
-
Notifications
You must be signed in to change notification settings - Fork 746
Feat/preview UI redesign #338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
3cafa97
5c28bf5
a737552
55af1de
be64eea
3f896ff
d75d40e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,7 @@ let previewShownFired = false; | |||||||||||||||||||
| let onRender: (() => void) | undefined; | ||||||||||||||||||||
| let trackUiEvent: ((event: string, params?: Record<string, unknown>) => void) | undefined; | ||||||||||||||||||||
| let rpcCallTool: ((name: string, args: Record<string, unknown>) => Promise<unknown>) | undefined; | ||||||||||||||||||||
| let rpcUpdateContext: ((text: string) => void) | undefined; | ||||||||||||||||||||
| let shellController: ToolShellController | undefined; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function getFileExtensionForAnalytics(filePath: string): string { | ||||||||||||||||||||
|
|
@@ -195,6 +196,13 @@ function stripReadStatusLine(content: string): string { | |||||||||||||||||||
| return content.replace(/^\[Reading [^\]]+\]\r?\n?/, ''); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function countContentLines(content: string): number { | ||||||||||||||||||||
| const cleaned = stripReadStatusLine(content); | ||||||||||||||||||||
| if (cleaned === '') return 0; | ||||||||||||||||||||
| const lines = cleaned.split('\n'); | ||||||||||||||||||||
| return lines[lines.length - 1] === '' ? lines.length - 1 : lines.length; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| interface ReadRange { | ||||||||||||||||||||
| fromLine: number; | ||||||||||||||||||||
| toLine: number; | ||||||||||||||||||||
|
|
@@ -218,7 +226,7 @@ function parseReadRange(content: string): ReadRange | undefined { | |||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function renderBody(payload: PreviewStructuredContent, htmlMode: HtmlPreviewMode): { html: string; notice?: string } { | ||||||||||||||||||||
| function renderBody(payload: PreviewStructuredContent, htmlMode: HtmlPreviewMode, startLine = 1): { html: string; notice?: string } { | ||||||||||||||||||||
| const cleanedContent = stripReadStatusLine(payload.content); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (payload.fileType === 'unsupported') { | ||||||||||||||||||||
|
|
@@ -237,7 +245,7 @@ function renderBody(payload: PreviewStructuredContent, htmlMode: HtmlPreviewMode | |||||||||||||||||||
| const formatted = formatJsonIfPossible(cleanedContent, payload.filePath); | ||||||||||||||||||||
| return { | ||||||||||||||||||||
| notice: formatted.notice, | ||||||||||||||||||||
| html: `<div class="panel-content source-content">${renderCodeViewer(formatted.content, detectedLanguage)}</div>` | ||||||||||||||||||||
| html: `<div class="panel-content source-content">${renderCodeViewer(formatted.content, detectedLanguage, startLine)}</div>` | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -351,6 +359,192 @@ function attachOpenInFolderHandler(payload: PreviewStructuredContent): void { | |||||||||||||||||||
| }); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function attachLoadAllHandler( | ||||||||||||||||||||
| container: HTMLElement, | ||||||||||||||||||||
| payload: PreviewStructuredContent, | ||||||||||||||||||||
| htmlMode: HtmlPreviewMode | ||||||||||||||||||||
| ): void { | ||||||||||||||||||||
| const beforeBtn = document.getElementById('load-before') as HTMLButtonElement | null; | ||||||||||||||||||||
| const afterBtn = document.getElementById('load-after') as HTMLButtonElement | null; | ||||||||||||||||||||
| if (!beforeBtn && !afterBtn) { | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const range = parseReadRange(payload.content); | ||||||||||||||||||||
| if (!range?.isPartial) return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const currentContent = stripReadStatusLine(payload.content); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const loadLines = async (btn: HTMLButtonElement, direction: 'before' | 'after'): Promise<void> => { | ||||||||||||||||||||
| const originalText = btn.textContent; | ||||||||||||||||||||
| btn.textContent = 'Loading…'; | ||||||||||||||||||||
| btn.disabled = true; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| trackUiEvent?.(direction === 'before' ? 'load_lines_before' : 'load_lines_after', { | ||||||||||||||||||||
| file_type: payload.fileType, | ||||||||||||||||||||
| file_extension: getFileExtensionForAnalytics(payload.filePath) | ||||||||||||||||||||
| }); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| try { | ||||||||||||||||||||
| // Load only the missing portion | ||||||||||||||||||||
| const readArgs = direction === 'before' | ||||||||||||||||||||
| ? { path: payload.filePath, offset: 0, length: range.fromLine - 1 } | ||||||||||||||||||||
| : { path: payload.filePath, offset: range.toLine }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const result = await rpcCallTool?.('read_file', readArgs); | ||||||||||||||||||||
| const resultObj = result as { content?: Array<{ text?: string }> } | undefined; | ||||||||||||||||||||
| const newText = resultObj?.content?.[0]?.text; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (newText && typeof newText === 'string') { | ||||||||||||||||||||
| const cleanNew = stripReadStatusLine(newText); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Merge: prepend or append the new lines | ||||||||||||||||||||
| const merged = direction === 'before' | ||||||||||||||||||||
| ? cleanNew + (cleanNew.endsWith('\n') ? '' : '\n') + currentContent | ||||||||||||||||||||
| : currentContent + (currentContent.endsWith('\n') ? '' : '\n') + cleanNew; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Build updated status line reflecting the new range | ||||||||||||||||||||
| const newFrom = direction === 'before' ? 1 : range.fromLine; | ||||||||||||||||||||
| const newTo = direction === 'after' ? range.totalLines : range.toLine; | ||||||||||||||||||||
| const lineCount = newTo - newFrom + 1; | ||||||||||||||||||||
| const remaining = range.totalLines - newTo; | ||||||||||||||||||||
| const isStillPartial = newFrom > 1 || newTo < range.totalLines; | ||||||||||||||||||||
| const statusLine = isStillPartial | ||||||||||||||||||||
| ? `[Reading ${lineCount} lines from ${newFrom === 1 ? 'start' : `line ${newFrom}`} (total: ${range.totalLines} lines, ${remaining} remaining)]\n` | ||||||||||||||||||||
| : ''; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const mergedPayload: PreviewStructuredContent = { | ||||||||||||||||||||
| ...payload, | ||||||||||||||||||||
| content: statusLine + merged | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| renderApp(container, mergedPayload, htmlMode, isExpanded); | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| btn.textContent = 'Failed to load'; | ||||||||||||||||||||
| setTimeout(() => { btn.textContent = originalText; btn.disabled = false; }, 2000); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } catch { | ||||||||||||||||||||
| btn.textContent = 'Failed to load'; | ||||||||||||||||||||
| setTimeout(() => { btn.textContent = originalText; btn.disabled = false; }, 2000); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| beforeBtn?.addEventListener('click', () => void loadLines(beforeBtn, 'before')); | ||||||||||||||||||||
| afterBtn?.addEventListener('click', () => void loadLines(afterBtn, 'after')); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Tracks native text selection and pushes it to the host via ui/update-model-context. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * How it works: | ||||||||||||||||||||
| * 1. User drags to select text anywhere in the preview (markdown, code, HTML). | ||||||||||||||||||||
| * 2. The selectionchange event fires; we extract the selected string. | ||||||||||||||||||||
| * 3. We call rpcUpdateContext() which sends a ui/update-model-context JSON-RPC | ||||||||||||||||||||
| * request to the host with the selected text + file path (+ line numbers for code). | ||||||||||||||||||||
| * 4. The host stores this as widget context. | ||||||||||||||||||||
| * 5. The LLM can access it by calling read_widget_context(tool_name="desktop-commander:read_file"). | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * Note: as of Feb 2025, Claude does NOT auto-inject ui/update-model-context into | ||||||||||||||||||||
| * the LLM's context window. The LLM must actively call read_widget_context to see | ||||||||||||||||||||
| * the selection. A floating tooltip near the selection tells the user this is working. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| function attachTextSelectionHandler(payload: PreviewStructuredContent): void { | ||||||||||||||||||||
| const contentWrapper = document.querySelector('.panel-content-wrapper') as HTMLElement | null; | ||||||||||||||||||||
| if (!contentWrapper) return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let hintEl: HTMLElement | null = null; | ||||||||||||||||||||
| let lastSelectedText = ''; | ||||||||||||||||||||
| let hideTimer: ReturnType<typeof setTimeout> | null = null; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function positionHint(selection: Selection): void { | ||||||||||||||||||||
| if (!hintEl) return; | ||||||||||||||||||||
| const range = selection.getRangeAt(0); | ||||||||||||||||||||
| const rect = range.getBoundingClientRect(); | ||||||||||||||||||||
| const wrapperRect = contentWrapper!.getBoundingClientRect(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Position above the selection, centered horizontally | ||||||||||||||||||||
| let left = rect.left + rect.width / 2 - wrapperRect.left; | ||||||||||||||||||||
| let top = rect.top - wrapperRect.top + contentWrapper!.scrollTop - 32; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Clamp within wrapper bounds | ||||||||||||||||||||
| const hintWidth = hintEl.offsetWidth || 200; | ||||||||||||||||||||
| left = Math.max(8, Math.min(left - hintWidth / 2, contentWrapper!.clientWidth - hintWidth - 8)); | ||||||||||||||||||||
| top = Math.max(4, top); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| hintEl.style.left = `${left}px`; | ||||||||||||||||||||
| hintEl.style.top = `${top}px`; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function showHint(selection: Selection): void { | ||||||||||||||||||||
| if (hideTimer) { clearTimeout(hideTimer); hideTimer = null; } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (!hintEl) { | ||||||||||||||||||||
| hintEl = document.createElement('div'); | ||||||||||||||||||||
| hintEl.className = 'selection-hint'; | ||||||||||||||||||||
| hintEl.textContent = 'AI can see your selection'; | ||||||||||||||||||||
| contentWrapper!.appendChild(hintEl); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| hintEl.classList.add('visible'); | ||||||||||||||||||||
| positionHint(selection); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function hideHint(): void { | ||||||||||||||||||||
| if (!hintEl) return; | ||||||||||||||||||||
| hintEl.classList.remove('visible'); | ||||||||||||||||||||
| hideTimer = setTimeout(() => { hintEl?.remove(); hintEl = null; }, 200); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function getLineInfo(selection: Selection): string { | ||||||||||||||||||||
| const anchorRow = selection.anchorNode?.parentElement?.closest('.code-line') as HTMLElement | null; | ||||||||||||||||||||
| const focusRow = selection.focusNode?.parentElement?.closest('.code-line') as HTMLElement | null; | ||||||||||||||||||||
| if (anchorRow && focusRow) { | ||||||||||||||||||||
| const a = parseInt(anchorRow.dataset.line ?? '', 10); | ||||||||||||||||||||
| const f = parseInt(focusRow.dataset.line ?? '', 10); | ||||||||||||||||||||
| if (!isNaN(a) && !isNaN(f)) { | ||||||||||||||||||||
| const low = Math.min(a, f); | ||||||||||||||||||||
| const high = Math.max(a, f); | ||||||||||||||||||||
| return low === high ? `line ${low}` : `lines ${low}–${high}`; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return ''; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| document.addEventListener('selectionchange', () => { | ||||||||||||||||||||
| const selection = document.getSelection(); | ||||||||||||||||||||
| if (!selection || selection.isCollapsed) { | ||||||||||||||||||||
| if (lastSelectedText) { | ||||||||||||||||||||
| lastSelectedText = ''; | ||||||||||||||||||||
| rpcUpdateContext?.(''); | ||||||||||||||||||||
| hideHint(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const text = selection.toString().trim(); | ||||||||||||||||||||
| if (!text || text === lastSelectedText) return; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Only act on selections within our content area | ||||||||||||||||||||
| const anchorInContent = contentWrapper!.contains(selection.anchorNode); | ||||||||||||||||||||
| const focusInContent = contentWrapper!.contains(selection.focusNode); | ||||||||||||||||||||
| if (!anchorInContent && !focusInContent) return; | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The selection hint and model context are never cleared when the user drags a new selection entirely outside the preview area (non-collapsed selection), so the floating "AI can see your selection" tooltip remains stuck near the old selection and the host continues to hold stale context, which is confusing and incorrect behavior. [logic error] Severity Level: Major
|
||||||||||||||||||||
| if (!anchorInContent && !focusInContent) return; | |
| if (!anchorInContent && !focusInContent) { | |
| if (lastSelectedText) { | |
| lastSelectedText = ''; | |
| rpcUpdateContext?.(''); | |
| hideHint(); | |
| } | |
| return; | |
| } |
Steps of Reproduction ✅
1. Open a file preview so `renderApp()` runs at `src/ui/file-preview/src/app.ts:579-713`
and renders the panel; this calls `attachTextSelectionHandler(payload)` at line 668, which
registers a `selectionchange` listener on `document` at
`src/ui/file-preview/src/app.ts:520-553`.
2. Inside the preview's `.panel-content-wrapper` (rendered in `renderApp()` at lines
636-657), select some code or markdown text; in the `selectionchange` handler, `selection`
is non-collapsed, `text` is non-empty, `anchorInContent`/`focusInContent` are true (lines
534-537), so `lastSelectedText` is set (line 539), `rpcUpdateContext` is called with a
context string (line 545) and the floating "AI can see your selection" hint is shown via
`showHint(selection)` (lines 486-497).
3. Without clicking to clear the selection, drag a new selection entirely outside
`.panel-content-wrapper` but still within the iframe document (e.g. select text in a
header or other UI above the panel); on the next `selectionchange`, `selection` is still
non-collapsed and `text` is non-empty (lines 521-532), but both `anchorInContent` and
`focusInContent` are now false for the preview content wrapper (line 535-536).
4. Because the code at lines 534-537 simply `return`s when `!anchorInContent &&
!focusInContent`, the handler exits without clearing `lastSelectedText`, without calling
`rpcUpdateContext?.('')`, and without invoking `hideHint()`. The stale tooltip remains
near the old selection inside the preview, and the host continues to hold the previous
selection text in `ui/update-model-context` until the selection is collapsed or the widget
is re-rendered.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/app.ts
**Line:** 537:537
**Comment:**
*Logic Error: The selection hint and model context are never cleared when the user drags a new selection entirely outside the preview area (non-collapsed selection), so the floating "AI can see your selection" tooltip remains stuck near the old selection and the host continues to hold stale context, which is confusing and incorrect behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off-by-one: "Load after" banner won't appear when exactly 1 line is missing.
(range.totalLines - range.toLine) > 1 requires more than 1 missing line. If a file has 100 total lines and lines 1–99 are loaded, 100 - 99 = 1, which fails the > 1 check, so no banner is shown and the user cannot load the final line.
Should this be > 0 (or >= 1)?
- const hasMissingAfter = range?.isPartial && range.toLine < range.totalLines && (range.totalLines - range.toLine) > 1;
+ const hasMissingAfter = range?.isPartial && range.toLine < range.totalLines;📝 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 hasMissingBefore = range?.isPartial && range.fromLine > 1; | |
| const hasMissingAfter = range?.isPartial && range.toLine < range.totalLines && (range.totalLines - range.toLine) > 1; | |
| const hasMissingBefore = range?.isPartial && range.fromLine > 1; | |
| const hasMissingAfter = range?.isPartial && range.toLine < range.totalLines; |
🤖 Prompt for AI Agents
In `@src/ui/file-preview/src/app.ts` around lines 618 - 619, The check for showing
the "load after" banner is off-by-one: update the hasMissingAfter condition
(currently using (range.totalLines - range.toLine) > 1) to treat any positive
missing count as missing (use > 0 or >= 1) so that when exactly one line remains
it still shows the banner; adjust the expression where hasMissingAfter is
computed alongside hasMissingBefore and keep the existing guards
(range?.isPartial) and property names (range.fromLine, range.toLine,
range.totalLines) intact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The hasMissingAfter condition now requires at least two lines to be missing at the end of the file before showing the "Load lines" banner; when exactly one line is missing (e.g., total 100 lines, previewing 1–99), no "load after" button is rendered, so the user can never load or see the final line of the file via the UI. [logic error]
Severity Level: Major ⚠️
- ⚠️ File preview cannot load final line in some partial views.
- ⚠️ Users may miss important trailing config or code lines.| const hasMissingAfter = range?.isPartial && range.toLine < range.totalLines && (range.totalLines - range.toLine) > 1; | |
| const hasMissingAfter = range?.isPartial && range.toLine < range.totalLines; |
Steps of Reproduction ✅
1. Open Desktop Commander and trigger any tool that returns a `PreviewStructuredContent`
payload for the file preview app (handled in `bootstrapApp()` at
`src/ui/file-preview/src/app.ts:715-777`), so that `renderApp()` is called at
`src/ui/file-preview/src/app.ts:579` with a `payload` representing a file.
2. Ensure the tool's content string represents a partial read of a file where exactly one
line is missing at the end, e.g. the text begins with `[Reading 99 lines from start
(total: 100 lines, 1 remaining)]` followed by lines 1–99 of a 100‑line file. This matches
the `parseReadRange()` pattern at `src/ui/file-preview/src/app.ts:213-226` and yields `{
fromLine: 1, toLine: 99, totalLines: 100, isPartial: true }`.
3. In `renderApp()` (`src/ui/file-preview/src/app.ts:595-613`), `range.isPartial` is true
and `range.toLine < range.totalLines` (99 < 100) but `(range.totalLines - range.toLine)`
equals `1`, so the `hasMissingAfter` condition at line 628 (`const hasMissingAfter =
range?.isPartial && range.toLine < range.totalLines && (range.totalLines - range.toLine) >
1;`) evaluates to `false`.
4. Because `hasMissingAfter` is false, `loadAfterBanner` is rendered as an empty string at
lines 632–633 and no "↓ Load lines 100–100" button appears in the DOM, so
`attachLoadAllHandler()` at `src/ui/file-preview/src/app.ts:362-433` finds no
`#load-after` element and cannot attach any handler to load the final line; the user
cannot load or view line 100 via the preview UI.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/ui/file-preview/src/app.ts
**Line:** 628:628
**Comment:**
*Logic Error: The `hasMissingAfter` condition now requires at least two lines to be missing at the end of the file before showing the "Load lines" banner; when exactly one line is missing (e.g., total 100 lines, previewing 1–99), no "load after" button is rendered, so the user can never load or see the final line of the file via the UI.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,8 +61,21 @@ export function formatJsonIfPossible(content: string, filePath: string): { conte | |
| } | ||
| } | ||
|
|
||
| export function renderCodeViewer(code: string, language = 'text'): string { | ||
| export function renderCodeViewer(code: string, language = 'text', startLine = 1): string { | ||
| const normalizedLanguage = language || 'text'; | ||
| const highlighted = highlightSource(code, normalizedLanguage); | ||
| return `<pre class="code-viewer"><code class="hljs language-${normalizedLanguage}">${highlighted}</code></pre>`; | ||
|
|
||
| // Wrap each line with line number gutter | ||
| const lines = highlighted.split('\n'); | ||
| // Remove trailing empty line from trailing newline | ||
| if (lines.length > 0 && lines[lines.length - 1] === '') { | ||
| lines.pop(); | ||
| } | ||
|
|
||
| const lineHtml = lines.map((line, i) => { | ||
| const lineNum = startLine + i; | ||
| return `<tr class="code-line" data-line="${lineNum}"><td class="line-num" data-line="${lineNum}">${lineNum}</td><td class="line-content">${line || ' '}</td></tr>`; | ||
| }).join('\n'); | ||
|
Comment on lines
66
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, locate the highlightSource function
rg -n "highlightSource" src/ui/file-preview/src/components/code-viewer.ts -A 2 -B 2Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 645 🏁 Script executed: # Also search for the function definition
ast-grep --pattern $'function highlightSource($_, $_) {
$$$
}'Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 58 🏁 Script executed: # Look for where highlightSource is defined or imported
rg -n "highlightSource" src/ui/file-preview/src/components/ -B 3 -A 10Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 6270 🏁 Script executed: cat -n src/ui/file-preview/src/components/highlighting.ts | head -70 | tail -35Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 1511 🏁 Script executed: # Check if highlight.js can produce spans spanning newlines by looking at test files
find src/ui/file-preview -name "*.test.*" -o -name "*.spec.*" | head -10Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 58 🏁 Script executed: # Look for any tests related to code-viewer or highlighting
rg -l "code-viewer\|highlightSource" src/ui/file-preview --include="*.test.*" --include="*.spec.*"Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 517 🏁 Script executed: # Check the package.json to see what version of highlight.js is used
rg -A 5 "highlight.js" package.jsonRepository: wonderwhy-er/DesktopCommanderMCP Length of output: 243 🏁 Script executed: # Let's also check how the highlighted output is actually being used -
# the key is whether the split fragments are ever directly compared or validated
rg -n "split.*\\\\n" src/ui/file-preview/src/components/ -B 3 -A 5Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 855 🏁 Script executed: # Let's understand highlight.js behavior better by checking what the actual output looks like
# Search for any test files or examples that might show the output
find . -type f -name "*.ts" -o -name "*.js" | xargs grep -l "highlightSource\|hljs.highlight" | head -5Repository: wonderwhy-er/DesktopCommanderMCP Length of output: 238 🏁 Script executed: # Check if there are any example files or documentation about expected output
cat -n src/ui/file-preview/src/components/highlighting.tsRepository: wonderwhy-er/DesktopCommanderMCP Length of output: 2785 🏁 Script executed: # Look for any existing issues or comments about line splitting
rg -n "split.*line\|newline\|line break" src/ui/file-preview/src/components/code-viewer.tsRepository: wonderwhy-er/DesktopCommanderMCP Length of output: 58 🌐 Web query:
💡 Result: In Highlight.js v11 it’s normal for a single token (eg a multiline comment/string) to be emitted as one Practical fix: post-process after highlightingUse the plugin hook Example (splitting multiline comment spans across lines, adapted from a common line-numbering workaround): hljs.addPlugin({
'after:highlightElement': ({ el }) => {
// Split multiline comment spans so each line starts a new span
el.innerHTML = el.innerHTML.replace(
/<span class="hljs-comment">([\s\S]*?)<\/span>/g,
(m) => m.replace(/\r?\n/g, '\n<span class="hljs-comment">')
);
}
});This is the same basic approach used in line-number/table-row implementations: detect the If you only care about preserving line breaks (not per-line spans)Make sure you’re using Sources: Plugin hook and timing ( Splitting highlighted HTML on highlight.js v11 emits Fix by post-processing the highlighted HTML to close open Span-repair helperfunction splitWithSpanRepair(highlighted: string): string[] {
const raw = highlighted.split('\n');
const result: string[] = [];
let openTags: string[] = [];
for (const line of raw) {
let patched = openTags.join('') + line;
// Track open spans and remove them from the line
const opens = (line.match(/<span[^>]*>/g) || []);
const closes = (line.match(/<\/span>/g) || []).length;
// Close any remaining open spans at line end
openTags = opens.slice(closes);
patched += '</span>'.repeat(Math.max(0, opens.length - closes));
result.push(patched);
}
return result;
}Then replace 🤖 Prompt for AI Agents |
||
|
|
||
| return `<pre class="code-viewer"><table class="code-table"><tbody>${lineHtml}</tbody></table></pre>`; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Load after" assumes the second
read_filefetches all remaining lines.After the RPC call returns,
newTois set torange.totalLines(Line 408) and the status line is rebuilt to reflect the full range. However,read_fileitself may paginate large results, returning only a portion of the remaining lines. If that happens, the reconstructed status header will claim all lines are present while the content is still truncated.Consider parsing the returned
newTextfor its own[Reading …]header to determine the actual range that was fetched, rather than assuming it covers everything fromrange.toLinetorange.totalLines.🤖 Prompt for AI Agents