add baseline for linked editing#54315
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
andrewbranch
left a comment
andrewbranch
left a comment
There was a problem hiding this comment.
Didn't look too closely at the baseline generation but the result looks great!
DanielRosenwasser
left a comment
DanielRosenwasser
left a comment
There was a problem hiding this comment.
Mostly looks good, though there's some duplication and style nits to address
src/harness/fourslashImpl.ts
Outdated
| const n = inlineLinkedEditBaseline[inlineLinkedEditBaseline.length - 1]; | ||
| baselineContent += `[|/*${n.index}*/` + fileText.slice(n.start, n.end) + `|]` + fileText.slice(inlineLinkedEditBaseline[inlineLinkedEditBaseline.length - 1].end) + linkedEditInfoBaseline; |
There was a problem hiding this comment.
| const n = inlineLinkedEditBaseline[inlineLinkedEditBaseline.length - 1]; | |
| baselineContent += `[|/*${n.index}*/` + fileText.slice(n.start, n.end) + `|]` + fileText.slice(inlineLinkedEditBaseline[inlineLinkedEditBaseline.length - 1].end) + linkedEditInfoBaseline; | |
| const lastRange = inlineLinkedEditBaseline[inlineLinkedEditBaseline.length - 1]; | |
| baselineContent += `[|/*${lastRange.index}*/` + fileText.slice(lastRange.start, lastRange.end) + `|]` + fileText.slice(lastRange.end) + linkedEditInfoBaseline; |
There was a problem hiding this comment.
Actually, maybe instead of copying the same contents within the loop, you just loop on every element and then write
const sliceEnd = i + 1 < inlineLinkedEditBaseline.length ?
inlineLinkedEditBaseline[i + 1].start :
undefined;
const trailingText = fileText.slice(e.end, sliceEnd);
baselineContent += `[|/*${e.index}*/` + fileText.slice(e.start, e.end) + `|]` + sliceEnd;
src/harness/fourslashImpl.ts
Outdated
| return { baselineContent: baselineContent + activeFile.content + `\n\n--No linked edits found--`, offset }; | ||
| } | ||
|
|
||
| let inlineLinkedEditBaseline: { start: number, end: number, index: number }[] = []; |
There was a problem hiding this comment.
Is there any reason that this is named index if it's always initialized with the current offset? Maybe offsetForBaseline or something?
There was a problem hiding this comment.
Also, since it's an array
| let inlineLinkedEditBaseline: { start: number, end: number, index: number }[] = []; | |
| let inlineLinkedEditBaselines: { start: number, end: number, index: number }[] = []; |
There was a problem hiding this comment.
index is the number it will be labeled with in the baseline output, and it's initalized to offset since it made sense to me to just use the variable that's passed into the function. offset is incremented per list entry in linkedEditsByRange like an index.
|
@typescript-bot cherry-pick this to release-5.1 |
|
Heya @DanielRosenwasser, I've started to run the task to cherry-pick this into |
|
Hey @DanielRosenwasser, I've opened #54419 for you. |
…54419) Co-authored-by: Isabel Duan <[email protected]> Co-authored-by: Daniel Rosenwasser <[email protected]>
| </[|/*2*/Link|]>; | ||
|
|
||
| === 0 === | ||
| {"ranges":[{"start":13,"length":8},{"start":73,"length":8}],"wordPattern":"[a-zA-Z0-9:\\-\\._$]*"} |
There was a problem hiding this comment.
I think you can skip mentioning ranges here since they are already marked in the text above.
6 participants
The baseline lets us know exactly when linked editing does and doesn't occur, and it makes long tests more readable.