Mark file unsaved when a missing image is relocated - #1754
Conversation
Fixes solvespace#1238. When a linked image is missing and the user relocates it through the file dialog, the new path was stored in memory but SS.unsaved was never set, so closing the file did not prompt to save and the updated path was lost. Set SS.unsaved = true in ReloadLinkedImage() when the path changes. However, SolveSpaceUI::Load() calls AfterNewFile() after loading, which unconditionally resets unsaved to false, wiping the flag. So capture the unsaved state set during loading and restore it after AfterNewFile().
|
@AnibalPinto Thank you for this contribution! |
ruevs
left a comment
There was a problem hiding this comment.
Completely broken! I regret I was away from a computer and did not test this on time before it was merged :-(
| // Capture whether loading modified the document (e.g. the user relocated | ||
| // a missing image, which sets unsaved in ReloadLinkedImage) before | ||
| // AfterNewFile() resets it (issue #1238). | ||
| bool unsavedDuringLoad = unsaved; |
There was a problem hiding this comment.
@AnibalPinto, @phkahler this a very, very, very bad new bug.
Once one file is modified in a "session" usaved == true and stays true. After that every opened file is always "unsaved" even if the user does not modify it! Just keep opening different files (or the same one over and over) - you get the save dialog every time!
There was a problem hiding this comment.
@ruevs Oh man. I tried it and it worked. Didn't think to do subsequent open and closes. Do you see a quick fix (like a place to set unsaved = false) or do we need to revert?
There was a problem hiding this comment.
@phkahler it's 01:50 (AM) where I am... too sleepy tho think about it and debug it now. I may get a chance to look tomorrow, but then I'll be away from the "geek PC" for 10 days...
There was a problem hiding this comment.
@ruevs @phkahler sorry for causing the bug.
A possible solution:
diff --git a/src/file.cpp b/src/file.cpp
index f013e812..a9a9875b 100644
--- a/src/file.cpp
+++ b/src/file.cpp
@@ -473,6 +473,9 @@ void SolveSpaceUI::LoadUsingTable(const Platform::Path &filename, char *key, cha
}
bool SolveSpaceUI::LoadFromFile(const Platform::Path &filename, bool canCancel) {
+ // Never carry a dirty flag from a prior document into a newly opened file.
+ unsaved = false;
+
bool fileIsEmpty = true;
allConsistent = false;
fileLoadError = false;
diff --git a/src/solvespace.cpp b/src/solvespace.cpp
index b819e068..37464ef0 100644
--- a/src/solvespace.cpp
+++ b/src/solvespace.cpp
@@ -191,6 +191,9 @@ bool SolveSpaceUI::LoadAutosaveFor(const Platform::Path &filename) {
}
bool SolveSpaceUI::Load(const Platform::Path &filename) {
+ // A file load should never inherit the dirty state from an earlier sketch.
+ unsaved = false;
+
bool autosaveLoaded = LoadAutosaveFor(filename);
bool fileLoaded = autosaveLoaded || LoadFromFile(filename, /*canCancel=*/true);
if(fileLoaded) {
diff --git a/test/core/path/test.cpp b/test/core/path/test.cpp
index 90ff1245..7f534cc8 100644
--- a/test/core/path/test.cpp
+++ b/test/core/path/test.cpp
@@ -1,3 +1,4 @@
+#include "solvespace.h"
#include "harness.h"
using Platform::Path;
@@ -88,6 +89,20 @@ TEST_CASE(with_extension) {
CHECK_EQ_STR(Path::From("foo").WithExtension("baz").raw, "foo.baz");
}
+TEST_CASE(load_clears_unsaved_state) {
+ Platform::Path path = Platform::Path::CurrentDirectory().Join(Path::From("tmp_unsaved_reset.slvs"));
+
+ SS.NewFile();
+ SS.AfterNewFile();
+ CHECK_TRUE(SS.SaveToFile(path));
+
+ SS.unsaved = true;
+ CHECK_TRUE(SS.LoadFromFile(path));
+ CHECK_FALSE(SS.unsaved);
+
+ RemoveFile(path);
+}
+
TEST_CASE(parent) {
Path path;
path = Path::From("foo" S "bar");
|
Using a flag to tell what to do with another flag is rarely the right thing. |
Fixes #1238.
When a linked image is missing and the user relocates it through the file dialog, the new path was stored in memory but SS.unsaved was never set, so closing the file did not prompt to save and the updated path was lost.
Set SS.unsaved = true in ReloadLinkedImage() when the path changes. However, SolveSpaceUI::Load() calls AfterNewFile() after loading, which unconditionally resets unsaved to false, wiping the flag. So capture the unsaved state set during loading and restore it after AfterNewFile().