Skip to content
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

Fix: duplicate saving and stale webview. #513

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

tomilho
Copy link
Contributor

@tomilho tomilho commented Apr 11, 2024

Should fix #511. Took me a bit of time to figure out everything, but I think the solution is working. I also added some tests to triple check the work.

Copy link
Member

@connor4312 connor4312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one comment

src/hexDocument.ts Outdated Show resolved Hide resolved
Fixed the duplicate save by saving only the timeline from the unsaved index.
Fixed the duplicate insertion by only applying timeline from the unsaved index.
Fixed bytes not showing after saving by forcing the webview to reload its raw
data pages.
Fixed diskFileSize state being updated with diskFileSize + editsDelta instead
of only diskFileSize.
Fixed diskFileSize state not being updated after document save.
Added tests to triple check the fix works in all places that used the
previous editTimeline.
Changed editTimeline to AllEditTimeline to prevent confusion with the
newly added unsavedEditTimeline
@connor4312
Copy link
Member

Thanks!

connor4312
connor4312 previously approved these changes Apr 16, 2024
@vscodenpa vscodenpa added this to the April 2024 milestone Apr 16, 2024
@connor4312
Copy link
Member

Hm, it looks like there's a test failure

auto-merge was automatically disabled April 16, 2024 19:27

Head branch was pushed to by a user without write access

}
return Promise.all(
ops.map(({ data, offset }) => {
return fd.write(data, 0, data.byteLength, offset);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

@connor4312 connor4312 enabled auto-merge (squash) April 18, 2024 18:38
@connor4312 connor4312 merged commit 00c2123 into microsoft:main Apr 18, 2024
4 checks passed
@tomilho tomilho deleted the fix-timeline branch April 18, 2024 19:21
@jogo- jogo- mentioned this pull request Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switching tabs and back after saving a paste messes the editor.
4 participants