Skip to content

Commit

Permalink
search: fix faulty replacements in crlf files with lf regex
Browse files Browse the repository at this point in the history
Fixes #165700

Regex normalization was actually working fine (finally). The issue was
JS code was matching again to do the replacement. If the original regex
only was written for LF, but the file was CRLF, VS Code did not attempt
to normalize line feeds until it tried to pull the surrounding content
as well as a 'last resort'.

If the surrounding content included a match before the intended one,
then that would be used instead, resulting in the bug.

PR does a small style change to add early returns to avoid nesting.
  • Loading branch information
connor4312 committed Dec 5, 2022
1 parent 782e13b commit a40099f
Showing 1 changed file with 25 additions and 13 deletions.
38 changes: 25 additions & 13 deletions src/vs/workbench/contrib/search/common/searchModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,25 +109,37 @@ export class Match {

const fullMatchText = this.fullMatchText();
let replaceString = searchModel.replacePattern.getReplaceString(fullMatchText, searchModel.preserveCase);
if (replaceString !== null) {
return replaceString;
}

// If match string is not matching then regex pattern has a lookahead expression
if (replaceString === null) {
const fullMatchTextWithSurroundingContent = this.fullMatchText(true);
replaceString = searchModel.replacePattern.getReplaceString(fullMatchTextWithSurroundingContent, searchModel.preserveCase);

// Search/find normalize line endings - check whether \r prevents regex from matching
if (replaceString === null) {
const fullMatchTextWithoutCR = fullMatchTextWithSurroundingContent.replace(/\r\n/g, '\n');
replaceString = searchModel.replacePattern.getReplaceString(fullMatchTextWithoutCR, searchModel.preserveCase);
// Search/find normalize line endings - check whether \r prevents regex from matching
const fullMatchTextWithoutCR = fullMatchText.replace(/\r\n/g, '\n');
if (fullMatchTextWithoutCR !== fullMatchText) {
replaceString = searchModel.replacePattern.getReplaceString(fullMatchTextWithoutCR, searchModel.preserveCase);
if (replaceString !== null) {
return replaceString;
}
}

// Match string is still not matching. Could be unsupported matches (multi-line).
if (replaceString === null) {
replaceString = searchModel.replacePattern.pattern;
// If match string is not matching then regex pattern has a lookahead expression
const contextMatchTextWithSurroundingContent = this.fullMatchText(true);
replaceString = searchModel.replacePattern.getReplaceString(contextMatchTextWithSurroundingContent, searchModel.preserveCase);
if (replaceString !== null) {
return replaceString;
}

// Search/find normalize line endings, this time in full context
const contextMatchTextWithoutCR = contextMatchTextWithSurroundingContent.replace(/\r\n/g, '\n');
if (contextMatchTextWithoutCR !== contextMatchTextWithSurroundingContent) {
replaceString = searchModel.replacePattern.getReplaceString(contextMatchTextWithoutCR, searchModel.preserveCase);
if (replaceString !== null) {
return replaceString;
}
}

return replaceString;
// Match string is still not matching. Could be unsupported matches (multi-line).
return searchModel.replacePattern.pattern;
}

fullMatchText(includeSurrounding = false): string {
Expand Down

0 comments on commit a40099f

Please sign in to comment.