Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/diffs/src/editor/pieceTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1026,7 +1026,6 @@ export function buildSearchReplacementText(
const lineText = getLineText(position.line);
const lineStart = offsetAt({ line: position.line, character: 0 });
const relStart = matchStart - lineStart;
const matched = lineText.slice(relStart, relStart + (matchEnd - matchStart));

let pattern: RegExp;
try {
Expand All @@ -1039,9 +1038,15 @@ export function buildSearchReplacementText(
return searchParams.replaceText;
}

const re = new RegExp(pattern.source, pattern.flags.replace('g', ''));
const match = re.exec(matched);
if (match === null || match[0].length !== matched.length) {
// Re-run at the original line offset so lookaround can inspect context
// outside the matched range while captures still come from this exact hit.
pattern.lastIndex = relStart;
const match = pattern.exec(lineText);
if (
match === null ||
match.index !== relStart ||
match[0].length !== matchEnd - matchStart
) {
return searchParams.replaceText;
}
return expandReplaceString(searchParams.replaceText, match);
Expand Down
8 changes: 1 addition & 7 deletions packages/diffs/test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ order. If you touch one of these areas, consider adding the missing coverage:
- **IME composition deferrals** — IME/composition interaction with editing and
undo-coalescing is deferred and not covered.

## Known bugs pinned as `test.failing` (27)
## Known bugs pinned as `test.failing` (21)

- **EditStack coalescing** (3) — coalescing decisions compare a new edit against
whatever sits on top of the undo stack purely by geometry, with no state reset
Expand Down Expand Up @@ -148,9 +148,3 @@ order. If you touch one of these areas, consider adding the missing coverage:
computation can return a position strictly inside a CRLF pair (a character
beyond the line's own length) that the inverse offset-from-position
computation clamps away, so the round trip silently loses a column.
- **Search-replace capture expansion with lookaround** (3) — replacement-text
expansion re-executes the pattern against only the matched slice, so
lookaround context outside the slice is lost: a lookbehind whose context sits
before the slice, a lookahead whose context sits after the slice, and a
lookahead that re-matches shorter on the slice all fall back to inserting the
literal (unexpanded) replacement text.
60 changes: 21 additions & 39 deletions packages/diffs/test/editorSearchPanel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,47 +531,29 @@ describe('regex replace capture references', () => {
).toEqual(['$&-$1']);
});

// KNOWN BUG: buildSearchReplacementText re-executes the pattern against only
// the matched slice; a lookbehind's context sits before the slice, so the
// re-execution finds nothing, falls back to the raw replaceText, and the
// literal "$1" is inserted into the document.
test.failing(
'lookbehind context before the match still expands its captures',
() => {
expect(
replacementsFor(
'k77',
searchParams('(?<=k)(\\d+)', { replaceText: 'n$1' })
)
).toEqual(['n77']);
}
);
test('lookbehind context before the match still expands its captures', () => {
expect(
replacementsFor(
'k77',
searchParams('(?<=k)(\\d+)', { replaceText: 'n$1' })
)
).toEqual(['n77']);
});

// KNOWN BUG: a lookahead's context sits after the matched slice, so the
// slice-only re-execution fails and the unexpanded replaceText is inserted.
test.failing(
'lookahead context after the match still expands its captures',
() => {
expect(
replacementsFor(
'run!',
searchParams('(\\w+)(?=!)', { replaceText: '<$1>' })
)
).toEqual(['<run>']);
}
);
test('lookahead context after the match still expands its captures', () => {
expect(
replacementsFor(
'run!',
searchParams('(\\w+)(?=!)', { replaceText: '<$1>' })
)
).toEqual(['<run>']);
});

// KNOWN BUG: on the matched slice the lookahead re-matches SHORTER than the
// original match (the trailing context character is part of the slice), the
// full-length guard rejects it, and the literal "[$&]" is inserted.
test.failing(
'a lookahead that re-matches shorter on the slice still expands',
() => {
expect(
replacementsFor('ooo', searchParams('o+(?=o)', { replaceText: '[$&]' }))
).toEqual(['[oo]']);
}
);
test('a lookahead that re-matches shorter on the slice still expands', () => {
expect(
replacementsFor('ooo', searchParams('o+(?=o)', { replaceText: '[$&]' }))
).toEqual(['[oo]']);
});

test('a pattern that matches nowhere leaves the document untouched', async () => {
const host = mountReplaceHost('gray goose');
Expand Down