mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
fix(cli): recognize markdown fences only on matching lines (#13331)
* fix(cli): recognize markdown fences only on matching lines Fixes #13309 * perf(cli): avoid repeated markdown fence scans * fix(cli): guard fence scans and strengthen regressions
This commit is contained in:
1 parent
da19e5c283
commit
2eee9b7186
7 files changed
+263
-46
No files matched your search
@@ -199,7 +199,6 @@ export const legacyFilenames = [
|
||||
'LspConfigLoader',
|
||||
'LspConnectionFactory',
|
||||
'LspResponseNormalizer',
|
||||
'markdownUtilities',
|
||||
'matcherGrouping',
|
||||
'mcpCommand',
|
||||
'McpPromptLoader',
|
||||
|
||||
@@ -51,7 +51,7 @@ import type { UseHistoryManagerReturn } from './useHistoryManager.js';
|
||||
import type { HistoryItem, SlashCommandProcessorResult } from '../types.js';
|
||||
import { MessageType, StreamingState, ToolCallStatus } from '../types.js';
|
||||
import type { LoadedSettings } from '../../config/settings.js';
|
||||
import { findLastSafeSplitPoint } from '../utils/markdownUtilities.js';
|
||||
import { findLastSafeSplitPoint } from '../utils/markdown-utilities.js';
|
||||
import {
|
||||
MAX_INLINE_IMAGE_ENCODED_LENGTH,
|
||||
MAX_INLINE_IMAGES_PER_ITEM,
|
||||
@@ -178,9 +178,9 @@ vi.mock('./shellCommandProcessor.js', () => ({
|
||||
|
||||
vi.mock('./atCommandProcessor.js');
|
||||
|
||||
vi.mock('../utils/markdownUtilities.js', async (importOriginal) => {
|
||||
vi.mock('../utils/markdown-utilities.js', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('../utils/markdownUtilities.js')>();
|
||||
await importOriginal<typeof import('../utils/markdown-utilities.js')>();
|
||||
return {
|
||||
...actual,
|
||||
// Only the split-point chooser is mocked so tests can drive commit
|
||||
|
||||
@@ -110,7 +110,7 @@ import {
|
||||
findLastSafeSplitPoint,
|
||||
splitFencedMarkdown,
|
||||
getEnclosingFenceInfo,
|
||||
} from '../utils/markdownUtilities.js';
|
||||
} from '../utils/markdown-utilities.js';
|
||||
import { fitPendingSlice } from '../utils/pending-rendered-height.js';
|
||||
import { useStateAndRef } from './useStateAndRef.js';
|
||||
import { normalizePartList } from '../../utils/normalize-part-list.js';
|
||||
|
||||
@@ -13,6 +13,7 @@ import { renderMermaidVisual } from './mermaidVisualRenderer.js';
|
||||
import { RenderModeProvider } from '../contexts/RenderModeContext.js';
|
||||
import { getScreenBuffer } from '../selection/screen-buffer.js';
|
||||
import { getSelectedText } from '../selection/selection-text.js';
|
||||
import { splitFencedMarkdown } from './markdown-utilities.js';
|
||||
|
||||
function copiedFrame(stdout: NodeJS.WriteStream): string {
|
||||
const frame = getScreenBuffer(stdout)!.frame!;
|
||||
@@ -115,6 +116,37 @@ describe('<MarkdownDisplay />', () => {
|
||||
expect(lastFrame()).toMatchSnapshot();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['~~~', '```'],
|
||||
['```', '~~~'],
|
||||
])(
|
||||
'renders %s code across an inner %s line after a streaming split',
|
||||
(outer, inner) => {
|
||||
const text =
|
||||
`${outer}md\nHere:\n${inner}\nstill inside\n${outer}\nAfter.`.replace(
|
||||
/\n/g,
|
||||
eol,
|
||||
);
|
||||
const { after } = splitFencedMarkdown(text, text.indexOf('still'));
|
||||
for (const renderedText of [text, after]) {
|
||||
for (const isPending of [false, true]) {
|
||||
const { lastFrame, unmount } = renderWithProviders(
|
||||
<MarkdownDisplay
|
||||
{...baseProps}
|
||||
text={renderedText}
|
||||
isPending={isPending}
|
||||
/>,
|
||||
);
|
||||
const frame = stripAnsi(lastFrame() ?? '');
|
||||
expect(frame).toMatch(/3\s+still inside/);
|
||||
expect(frame).toContain('After.');
|
||||
expect(frame).not.toContain('qwen-code');
|
||||
unmount();
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it('clips a long pending message to availableTerminalHeight', () => {
|
||||
// A long streaming message must NOT render all its lines: the live
|
||||
// (non-<Static>) frame would exceed the terminal height and ink would
|
||||
|
||||
@@ -16,7 +16,7 @@ import { useSettings } from '../contexts/SettingsContext.js';
|
||||
import { MermaidDiagram } from './MermaidDiagram.js';
|
||||
import { renderInlineLatex } from './latexRenderer.js';
|
||||
import { useRenderMode } from '../contexts/RenderModeContext.js';
|
||||
import { parseCodeFenceInfo } from './markdownUtilities.js';
|
||||
import { parseCodeFenceInfo } from './markdown-utilities.js';
|
||||
import {
|
||||
fitPendingSlice,
|
||||
splitMarkdownTableRow,
|
||||
|
||||
+167
-3
@@ -4,15 +4,16 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { CODE_FENCE_RE } from './pending-rendered-height.js';
|
||||
import {
|
||||
findLastSafeSplitPoint,
|
||||
splitFencedMarkdown,
|
||||
parseCodeFenceInfo,
|
||||
getEnclosingFenceInfo,
|
||||
} from './markdownUtilities.js';
|
||||
} from './markdown-utilities.js';
|
||||
|
||||
describe('markdownUtilities', () => {
|
||||
describe('markdown-utilities', () => {
|
||||
describe('findLastSafeSplitPoint', () => {
|
||||
it('should split at the last double newline if not in a code block', () => {
|
||||
const content = 'paragraph1\n\nparagraph2\n\nparagraph3';
|
||||
@@ -91,6 +92,57 @@ describe('markdownUtilities', () => {
|
||||
const content = '```ts\n' + 'a'.repeat(100);
|
||||
expect(findLastSafeSplitPoint(content, 40)).toBe(40);
|
||||
});
|
||||
|
||||
it('re-arms the closed-block walk for the single-newline fallback', () => {
|
||||
const content = '```a\nx\n\ny\n```\n```b\nz\n```TAIL';
|
||||
expect(findLastSafeSplitPoint(content, 22)).toBe(14);
|
||||
});
|
||||
|
||||
it.each(['```', '``````', '~~~'])(
|
||||
'skips the fence regex for a backtick after the full %s delimiter',
|
||||
(delimiter) => {
|
||||
const content = delimiter + ' '.repeat(20000) + '`\n\nafter';
|
||||
const scan = vi.spyOn(CODE_FENCE_RE, 'exec');
|
||||
try {
|
||||
expect(findLastSafeSplitPoint(content, 16384)).toBe(16384);
|
||||
expect(scan.mock.calls.length).toBe(0);
|
||||
} finally {
|
||||
scan.mockRestore();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each([200, 2000])(
|
||||
'scans fences once when rejecting %s internal blank lines',
|
||||
(records) => {
|
||||
const fenced = `\`\`\`csv\n${'x\n\n'.repeat(records)}\`\`\`\n`;
|
||||
const content = fenced + 'a'.repeat(16500 - fenced.length);
|
||||
const scan = vi.spyOn(CODE_FENCE_RE, 'exec');
|
||||
try {
|
||||
expect(findLastSafeSplitPoint(content, 16384)).toBe(fenced.length);
|
||||
expect(scan.mock.calls.length).toBeGreaterThan(0);
|
||||
expect(scan.mock.calls.length).toBeLessThanOrEqual(4);
|
||||
} finally {
|
||||
scan.mockRestore();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it('visits each fence once across many closed blocks', () => {
|
||||
const blocks = 1000;
|
||||
const fenced = '```csv\nx\n\n```\n'.repeat(blocks);
|
||||
const content = fenced + 'a'.repeat(100);
|
||||
const scan = vi.spyOn(CODE_FENCE_RE, 'exec');
|
||||
try {
|
||||
expect(findLastSafeSplitPoint(content, fenced.length + 10)).toBe(
|
||||
fenced.length,
|
||||
);
|
||||
expect(scan.mock.calls.length).toBeGreaterThan(0);
|
||||
expect(scan.mock.calls.length).toBeLessThanOrEqual(blocks * 2);
|
||||
} finally {
|
||||
scan.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitFencedMarkdown', () => {
|
||||
@@ -283,4 +335,116 @@ describe('markdownUtilities', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('fence recognition', () => {
|
||||
it.each([
|
||||
['~~~', '```'],
|
||||
['```', '~~~'],
|
||||
])('keeps %s open across an inner %s line', (outer, inner) => {
|
||||
const opening = `${outer}md\nHere:\n${inner}\n`;
|
||||
const pending = `${opening}still inside\n`;
|
||||
const content = `${pending}${outer}\nAfter.\n`;
|
||||
const splitPoint = content.indexOf('still');
|
||||
|
||||
for (const text of [pending, content]) {
|
||||
expect(getEnclosingFenceInfo(text, splitPoint)).toEqual({
|
||||
lang: 'md',
|
||||
startLine: 1,
|
||||
});
|
||||
expect(findLastSafeSplitPoint(text, splitPoint)).toBe(splitPoint);
|
||||
expect(splitFencedMarkdown(text, splitPoint)).toEqual({
|
||||
before: `${opening}${outer}\n`,
|
||||
after: `${outer}md qwen-code:start-line=3\n${text.slice(splitPoint)}`,
|
||||
});
|
||||
}
|
||||
expect(
|
||||
getEnclosingFenceInfo(content, content.indexOf('After')),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it.each(['~~~x', '~~~~~x'])(
|
||||
'advances past a same-character fence marker in the info string: %s',
|
||||
(info) => {
|
||||
const content = `~~~ ${info}\ncode\n~~~\nAfter`;
|
||||
expect(getEnclosingFenceInfo(content, content.indexOf('code'))).toEqual(
|
||||
{
|
||||
lang: info,
|
||||
startLine: 1,
|
||||
},
|
||||
);
|
||||
expect(
|
||||
getEnclosingFenceInfo(content, content.indexOf('After')),
|
||||
).toBeNull();
|
||||
},
|
||||
);
|
||||
|
||||
it('advances past a fence info string containing the other fence marker', () => {
|
||||
const content = '``` ~~~\ncode\n```\nAfter';
|
||||
expect(getEnclosingFenceInfo(content, content.indexOf('code'))).toEqual({
|
||||
lang: '~~~',
|
||||
startLine: 1,
|
||||
});
|
||||
expect(
|
||||
getEnclosingFenceInfo(content, content.indexOf('After')),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['~~~', 'Use `~~~` as the fence marker.'],
|
||||
['```', 'Use ``` as the fence marker.'],
|
||||
])('keeps an inline %s marker in ordinary prose', (_marker, intro) => {
|
||||
const content = `${intro}\nMore ordinary prose follows.`;
|
||||
const splitPoint = content.indexOf('More');
|
||||
|
||||
expect(getEnclosingFenceInfo(content, splitPoint)).toBeNull();
|
||||
expect(findLastSafeSplitPoint(content, splitPoint + 8)).toBe(splitPoint);
|
||||
expect(splitFencedMarkdown(content, splitPoint)).toEqual({
|
||||
before: `${intro}\n`,
|
||||
after: 'More ordinary prose follows.',
|
||||
});
|
||||
});
|
||||
|
||||
it.each(['```ts`invalid', '~~~ts`invalid'])(
|
||||
'rejects an opening fence with a backtick in its info string: %s',
|
||||
(opening) => {
|
||||
const content = `${opening}\nMore ordinary prose follows.`;
|
||||
const splitPoint = content.indexOf('More');
|
||||
|
||||
expect(getEnclosingFenceInfo(content, splitPoint)).toBeNull();
|
||||
expect(findLastSafeSplitPoint(content, splitPoint + 8)).toBe(
|
||||
splitPoint,
|
||||
);
|
||||
expect(splitFencedMarkdown(content, splitPoint)).toEqual({
|
||||
before: `${opening}\n`,
|
||||
after: 'More ordinary prose follows.',
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(['````', '~~~~'])(
|
||||
'recognizes an indented %s fence after ordinary prose',
|
||||
(delimiter) => {
|
||||
const intro = 'Use `~~~` as the fence marker.\n';
|
||||
const content = `${intro} ${delimiter}ts\nline1\nline2\n ${delimiter}\n\nAfter prose.`;
|
||||
const splitPoint = content.indexOf('line2');
|
||||
const afterBlock = content.indexOf('After');
|
||||
|
||||
expect(getEnclosingFenceInfo(content, splitPoint)).toEqual({
|
||||
lang: 'ts',
|
||||
startLine: 1,
|
||||
});
|
||||
expect(findLastSafeSplitPoint(content, splitPoint)).toBe(
|
||||
intro.length + 2,
|
||||
);
|
||||
expect(splitFencedMarkdown(content, splitPoint)).toEqual({
|
||||
before: `${intro} ${delimiter}ts\nline1\n${delimiter}\n`,
|
||||
after: `${delimiter}ts qwen-code:start-line=2\nline2\n ${delimiter}\n\nAfter prose.`,
|
||||
});
|
||||
expect(getEnclosingFenceInfo(content, afterBlock)).toBeNull();
|
||||
expect(findLastSafeSplitPoint(content, afterBlock + 3)).toBe(
|
||||
afterBlock,
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
+59
-37
@@ -4,6 +4,8 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { CODE_FENCE_RE } from './pending-rendered-height.js';
|
||||
|
||||
/*
|
||||
**Background & Purpose:**
|
||||
|
||||
@@ -40,9 +42,9 @@ so rendered history chunks do not break fenced code blocks unnecessarily.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Finds the next fenced-code delimiter (a run of 3+ ``` or ~~~) at or after
|
||||
* `from`, returning its index, fence character and the FULL run length. Both
|
||||
* fence types are recognized. The run length matters: `indexOf('```')` matches
|
||||
* Finds the next standalone fenced-code delimiter at or after `from`, using
|
||||
* MarkdownDisplay's line-matching rules and returning its index, fence character
|
||||
* and the FULL run length. The run length matters: `indexOf('```')` matches
|
||||
* only the first 3 chars of a longer run, so callers must advance past the whole
|
||||
* run (index + length) or a 6-backtick fence would be miscounted as two.
|
||||
*/
|
||||
@@ -50,21 +52,39 @@ const findNextFence = (
|
||||
content: string,
|
||||
from: number,
|
||||
): { index: number; char: '`' | '~'; length: number } | null => {
|
||||
const backtick = content.indexOf('```', from);
|
||||
const tilde = content.indexOf('~~~', from);
|
||||
if (backtick === -1 && tilde === -1) return null;
|
||||
let index: number;
|
||||
let char: '`' | '~';
|
||||
if (tilde === -1 || (backtick !== -1 && backtick < tilde)) {
|
||||
index = backtick;
|
||||
char = '`';
|
||||
} else {
|
||||
index = tilde;
|
||||
char = '~';
|
||||
let lineStart = from === 0 ? 0 : content.lastIndexOf('\n', from - 1) + 1;
|
||||
while (lineStart < content.length) {
|
||||
const newlineIndex = content.indexOf('\n', lineStart);
|
||||
const lineEnd = newlineIndex === -1 ? content.length : newlineIndex;
|
||||
// This pre-filter must remain a superset of CODE_FENCE_RE.
|
||||
let delimiterStart = lineStart;
|
||||
while (content[delimiterStart] === ' ') delimiterStart++;
|
||||
if (
|
||||
delimiterStart >= from &&
|
||||
(content.startsWith('```', delimiterStart) ||
|
||||
content.startsWith('~~~', delimiterStart))
|
||||
) {
|
||||
let delimiterEnd = delimiterStart + 3;
|
||||
while (content[delimiterEnd] === content[delimiterStart]) delimiterEnd++;
|
||||
if (content.slice(delimiterEnd, lineEnd).includes('`')) {
|
||||
lineStart = lineEnd + 1;
|
||||
continue;
|
||||
}
|
||||
const match = CODE_FENCE_RE.exec(content.slice(lineStart, lineEnd));
|
||||
if (!match) {
|
||||
lineStart = lineEnd + 1;
|
||||
continue;
|
||||
}
|
||||
const delimiter = match[1]!;
|
||||
return {
|
||||
index: delimiterStart,
|
||||
char: delimiter[0] === '`' ? '`' : '~',
|
||||
length: delimiter.length,
|
||||
};
|
||||
}
|
||||
lineStart = lineEnd + 1;
|
||||
}
|
||||
let length = 0;
|
||||
while (content[index + length] === char) length++;
|
||||
return { index, char, length };
|
||||
return null;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -80,6 +100,7 @@ const findNextFence = (
|
||||
const getEnclosingFence = (
|
||||
content: string,
|
||||
indexToTest: number,
|
||||
onClosed?: (startIndex: number, endIndex: number) => void,
|
||||
): { startIndex: number; delimiter: string; infoString: string } | null => {
|
||||
let open: { char: '`' | '~'; len: number; index: number } | null = null;
|
||||
let searchPos = 0;
|
||||
@@ -89,6 +110,7 @@ const getEnclosingFence = (
|
||||
if (!open) {
|
||||
open = { char: fence.char, len: fence.length, index: fence.index };
|
||||
} else if (fence.char === open.char && fence.length >= open.len) {
|
||||
onClosed?.(open.index, fence.index);
|
||||
open = null;
|
||||
}
|
||||
searchPos = fence.index + fence.length;
|
||||
@@ -103,20 +125,6 @@ const getEnclosingFence = (
|
||||
};
|
||||
};
|
||||
|
||||
/**
|
||||
* Checks if a given character index is inside a fenced code block (``` or ~~~).
|
||||
* Shares its fence-matching rules with {@link getEnclosingFence}.
|
||||
*/
|
||||
const isIndexInsideCodeBlock = (content: string, index: number): boolean =>
|
||||
getEnclosingFence(content, index) !== null;
|
||||
|
||||
/**
|
||||
* Finds the starting index of the code block (``` or ~~~) that encloses the
|
||||
* given index. Returns -1 if the index is not inside a code block.
|
||||
*/
|
||||
const findEnclosingCodeBlockStart = (content: string, index: number): number =>
|
||||
getEnclosingFence(content, index)?.startIndex ?? -1;
|
||||
|
||||
/**
|
||||
* When `index` sits inside an open fenced code block, returns that block's
|
||||
* language and gutter start line; otherwise null. Lets the streaming commit
|
||||
@@ -145,14 +153,27 @@ export const findLastSafeSplitPoint = (
|
||||
return content.length;
|
||||
}
|
||||
|
||||
const enclosingBlockStart = findEnclosingCodeBlockStart(content, searchEnd);
|
||||
if (enclosingBlockStart !== -1) {
|
||||
const closedBlocks: Array<{ start: number; end: number }> = [];
|
||||
const enclosingFence = getEnclosingFence(content, searchEnd, (start, end) => {
|
||||
closedBlocks.push({ start, end });
|
||||
});
|
||||
if (enclosingFence) {
|
||||
// The end of the content is contained in a code block. Split right before.
|
||||
return hasLengthCap && enclosingBlockStart === 0
|
||||
return hasLengthCap && enclosingFence.startIndex === 0
|
||||
? searchEnd
|
||||
: enclosingBlockStart;
|
||||
: enclosingFence.startIndex;
|
||||
}
|
||||
|
||||
// Candidates move backwards, so each closed block is visited at most once
|
||||
// rather than re-scanning all fences for every rejected newline.
|
||||
let blockIndex = closedBlocks.length - 1;
|
||||
const isInsideClosedBlock = (index: number): boolean => {
|
||||
while (blockIndex >= 0 && index <= closedBlocks[blockIndex]!.start) {
|
||||
blockIndex--;
|
||||
}
|
||||
return blockIndex >= 0 && index <= closedBlocks[blockIndex]!.end;
|
||||
};
|
||||
|
||||
// Search for the last double newline (\n\n) not in a code block.
|
||||
let searchStartIndex = searchEnd;
|
||||
while (searchStartIndex >= 0) {
|
||||
@@ -165,7 +186,7 @@ export const findLastSafeSplitPoint = (
|
||||
const potentialSplitPoint = dnlIndex + 2;
|
||||
if (
|
||||
potentialSplitPoint <= searchEnd &&
|
||||
!isIndexInsideCodeBlock(content, potentialSplitPoint)
|
||||
!isInsideClosedBlock(potentialSplitPoint)
|
||||
) {
|
||||
return potentialSplitPoint;
|
||||
}
|
||||
@@ -176,6 +197,7 @@ export const findLastSafeSplitPoint = (
|
||||
}
|
||||
|
||||
if (hasLengthCap) {
|
||||
blockIndex = closedBlocks.length - 1;
|
||||
searchStartIndex = searchEnd;
|
||||
while (searchStartIndex >= 0) {
|
||||
const nlIndex = content.lastIndexOf('\n', searchStartIndex);
|
||||
@@ -186,7 +208,7 @@ export const findLastSafeSplitPoint = (
|
||||
const potentialSplitPoint = nlIndex + 1;
|
||||
if (
|
||||
potentialSplitPoint <= searchEnd &&
|
||||
!isIndexInsideCodeBlock(content, potentialSplitPoint)
|
||||
!isInsideClosedBlock(potentialSplitPoint)
|
||||
) {
|
||||
return potentialSplitPoint;
|
||||
}
|
||||
Reference in new issue
Block a user