Skip to content

Do not save after ReplaceModifiedPart#75

Open
dizlexik wants to merge 1 commit intogornostal:masterfrom
dizlexik:patch-1
Open

Do not save after ReplaceModifiedPart#75
dizlexik wants to merge 1 commit intogornostal:masterfrom
dizlexik:patch-1

Conversation

@dizlexik
Copy link
Copy Markdown

This is not at all expected in my opinion and actually caused me to lose a bit of work earlier. And on top of it not being expected, it also does not update the tab's dirty state correctly. I really think auto-saving after this command is dangerous and unneeded.

@gornostal
Copy link
Copy Markdown
Owner

I see your point, but if we remove "save" users will face another problem:
If you do ReplaceModifiedPart several times, you may not get expected behavior, because actual diff is different from what is cached in Modific.

You can try it yourself, you'll see what I'm talking about.

I'll leave this pull request open for now. Maybe I'll disable this function if document is 'dirty'. Let me think about it.

@dizlexik
Copy link
Copy Markdown
Author

dizlexik commented Sep 4, 2014

Yeah I see what you're saying, that makes sense. I like the idea of disabling it if the document is dirty. And by the way, besides this issue the rest of the functionality in this package is fantastic and I love using it! I just don't want anyone else to lose work like I did, that's all.

@gornostal
Copy link
Copy Markdown
Owner

Yes, sure. I don't want that either.
I'm just curious, din't ctrl+z help you to revert changes? It should have worked.

I'll suggest you using your branch for now, while I'm trying to find time to fix the issue properly.

@dizlexik
Copy link
Copy Markdown
Author

dizlexik commented Sep 5, 2014

I didn't realize that it'd been saved so I just closed the tab expecting to select No when it asked whether or not I wanted to save changes. But since the tab's state wasn't dirty it just closed immediately and I lost my changes forever. But yeah I expect that ctrl+z would have worked had I known what was actually happening. It's just very counter-intuitive in my opinion, that's all.

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.

2 participants