Handle dirty state correctly when closing document
Fixes https://bugs.launchpad.net/pinta/+bug/1435226
The implementation was committing tool state first, which can finalize editable shapes and push a history item, marking the document dirty right before closing.
This was changed so that:
- If the document is not dirty, close it immediately without committing
- Only commit pending changes when the document is dirty and we might prompt to save
This prevents creating a last-second history item that triggers the prompt after a successful save.
Tested locally the fact that the bug was still reproducing before the change and does not reproduce after this. The image is still correctly saved after using any tool, saving and closing the app.
Thanks for investigating this! I agree we shouldn't be committing before checking the dirty state, since after saving the tool will have rolled back any history items added while committing, and the state will be marked as clean https://github.com/PintaProject/Pinta/blob/bfc905a6b6590391d6b08488e3c7742e7935d16a/Pinta/Actions/File/SaveDocumentImplementationAction.cs#L275
I'm thinking we should also go further though and remove the tools.Commit() entirely here - this already happens in the actual code for saving the file so it seems unnecessary to do it up front.
https://github.com/PintaProject/Pinta/blob/bfc905a6b6590391d6b08488e3c7742e7935d16a/Pinta/Actions/File/SaveDocumentImplementationAction.cs#L238
This would avoid adding the "Finalize" history item if the user decides to cancel closing the document, for example:
https://github.com/user-attachments/assets/7042604b-cbcc-48b5-962b-9d215016f899
Thank you! I didn't catch that detail. I wonder why this call was added in the CloseDocumentationAction class to begin with. Maybe as an extra precaution in case some tool doesn't call Commit.
With the call to tools.Commit () removed entirely everything still seems to work, but the Finalize state is still in the History when cancelling that save dialogue. I proposed a fix in https://github.com/PintaProject/Pinta/pull/1656/commits/87c57b688cf2c381732e690df77f1cd2e3ae34df, though there maybe could be a better way to handle that, but I couldn't figure out anything else.
While trying to replicate the example in your screen recording, I noticed that upon canceling or saving the current tab, the shape is Finalized and added to the other tab that is already opened. This is a bug that seems like it was there before the changes in this PR, but it seems like a pretty bad one. I proposed a fix in https://github.com/PintaProject/Pinta/pull/1656/commits/e8a281e3c849cc6fad12a54655e8630c7019b1ee
This fix and the fix for Finalize above can probably be split into a separate PR if needed.
So far, these are the changes in this PR:
- No Tool commit on close document prompt.
- Suppressed commit when switching active document during tab close.
- Ignored transient tab selection changes during close.
- Shape finalization and shape drawing and handles prevented on wrong tab.
It looks like there are some merge conflicts, so I suspect your branch is behind the latest master branch?
I wasn't able to reproduce the Finalize state is still in the History when cancelling that save dialogue issue, but perhaps I'm missing some steps or it was fixed by some other changes
Would you also maybe be able to record a screen grab of the other issue with shape is Finalized and added to the other tab? e.g. does the other document need to also have shapes in it?