-
Notifications
You must be signed in to change notification settings - Fork 110
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
sort the edits in descending order #289
base: master
Are you sure you want to change the base?
Conversation
fixes #288 > Applying the edit instructions in reverse to file will result in correctly reformatted text. One of the comments in TypeScript repo says the edits should be applied in reverse order. But, I am not sure all the plugins follows this? Doing a manual sort does seem to fix the tslint issue. VS code applies the fixes correctly, but I can't able to find the logic. Atom[1] also manually sorts the edits. 1: https://github.com/TypeStrong/atom-typescript/blob/master/lib/main/pluginManager.ts#L349-L352
@jupl could you try this branch? |
I can still replicate the issue unfortunately. :( |
Could you share a minimal project where I can reproduce the issue.
…On Sun, Nov 25, 2018, 12:35 AM jupl ***@***.*** wrote:
I can still replicate the issue unfortunately. :(
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#289 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJG9sF0Ll4WmdTIZ_ZUtXEnvU-9i9VGks5uyZhggaJpZM4YwJWo>
.
|
https://github.com/jupl/tide-thingy I can fix the |
* don't format after the application of code-edit * group the code-edits by filename, so the changes for the same file will be processed together.
Thanks for the example, it was helpful in debugging. I have added two more fixes
I am kind of not sure about merging these changes. AFAIK, these things are not thoroughly documented anywhere, so I am of not sure what is the correct fix and who is at fault here. |
/** Apply each textChange in order, updating future changes to account for the text offset of previous changes. */
function forEachTextChange(changes: ReadonlyArray<ts.TextChange>, cb: (change: ts.TextChange) => void): void {
// Copy this so we don't ruin someone else's copy
changes = JSON.parse(JSON.stringify(changes));
for (let i = 0; i < changes.length; i++) {
const change = changes[i];
cb(change);
const changeDelta = change.newText.length - change.span.length;
for (let j = i + 1; j < changes.length; j++) {
if (changes[j].span.start >= change.span.start) {
changes[j].span.start += changeDelta;
}
}
}
} from this logic it looks like applying the changes in reverse order should work, as there won't be any more span that will be affected by the current one |
@jupl could you test it once more |
fixes #288
One of the comments in TypeScript repo says the edits should be applied
in reverse order. But, I am not sure all the plugins follow this? Doing
a manual sort does seem to fix the tslint issue. VS code applies the fixes
correctly, but I can't able to find the logic. Atom[1] also manually
sorts the edits.
1: https://github.com/TypeStrong/atom-typescript/blob/master/lib/main/pluginManager.ts#L349-L352