From c123bc2bf83330994feb3e175f1080881e4a8e28 Mon Sep 17 00:00:00 2001 From: Ajinkya Dahale Date: Thu, 16 Sep 2021 13:06:29 -0400 Subject: [PATCH] [Core] (Partially?) Fix data loss on dir rename (#4996) * Fix lost filename in err msg In some circumstances, FileExceptions are constructed empty, then have a filename assigned to them, but the error message in these scenarios is left as the default "unknown" one, which is sometimes shown to users. This change fixes that case to be consistent with instances that are constructed with the filename. The exception can happen when trying to save the file in a location that does not exist, or when the user does not have permission to write there. If it comes when saving after closing the document, all previous changes can be lost. Partially fixes issue #4098. Co-authored-by: Heewa Barfchin --- src/App/Document.cpp | 5 ++++ src/Gui/Document.cpp | 68 +++++++++++++++++++++++++++++++++++++++--- src/Gui/MainWindow.cpp | 38 ++++++++++++++++------- src/Gui/MainWindow.h | 7 +++++ 4 files changed, 104 insertions(+), 14 deletions(-) diff --git a/src/App/Document.cpp b/src/App/Document.cpp index bcbf6b3849..aa099d216a 100644 --- a/src/App/Document.cpp +++ b/src/App/Document.cpp @@ -66,6 +66,7 @@ recompute path. Also, it enables more complicated dependencies beyond trees. # include # include # include +# include #endif #include @@ -142,6 +143,8 @@ using namespace zipios; # define FC_LOGFEATUREUPDATE #endif +namespace fs = boost::filesystem; + // typedef boost::property VertexProperty; typedef boost::adjacency_list < boost::vecS, // class OutEdgeListS : a Sequence or an AssociativeContainer @@ -2610,6 +2613,8 @@ bool Document::saveToFile(const char* filename) const fn += uuid; } Base::FileInfo tmp(fn); + // In case some folders in the path do not exist + fs::create_directories(fs::path(filename).parent_path()); // open extra scope to close ZipWriter properly { diff --git a/src/Gui/Document.cpp b/src/Gui/Document.cpp index 16c4fa5836..99e9bddf18 100644 --- a/src/Gui/Document.cpp +++ b/src/Gui/Document.cpp @@ -1158,6 +1158,25 @@ bool Document::save(void) if(gdoc) gdoc->setModified(false); } } + catch (const Base::FileException& e) { + int ret = QMessageBox::question( + getMainWindow(), + QObject::tr("Could not Save Document"), + QObject::tr("There was an issue trying to save the file. " + "This may be because some of the parent folders do not exist, " + "or you do not have sufficient permissions, " + "or for other reasons. Error details:\n\n\"%1\"\n\n" + "Would you like to save the file with a different name?") + .arg(QString::fromLatin1(e.what())), + QMessageBox::Yes, QMessageBox::No); + if (ret == QMessageBox::No) { + // TODO: Understand what exactly is supposed to be returned here + getMainWindow()->showMessage(QObject::tr("Saving aborted"), 2000); + return false; + } else if (ret == QMessageBox::Yes) { + return saveAs(); + } + } catch (const Base::Exception& e) { QMessageBox::critical(getMainWindow(), QObject::tr("Saving document failed"), QString::fromLatin1(e.what())); @@ -1196,6 +1215,25 @@ bool Document::saveAs(void) setModified(false); getMainWindow()->appendRecentFile(fi.filePath()); } + catch (const Base::FileException& e) { + int ret = QMessageBox::question( + getMainWindow(), + QObject::tr("Could not Save Document"), + QObject::tr("There was an issue trying to save the file. " + "This may be because some of the parent folders do not exist, " + "or you do not have sufficient permissions, " + "or for other reasons. Error details:\n\n\"%1\"\n\n" + "Would you like to save the file with a different name?") + .arg(QString::fromLatin1(e.what())), + QMessageBox::Yes, QMessageBox::No); + if (ret == QMessageBox::No) { + // TODO: Understand what exactly is supposed to be returned here + getMainWindow()->showMessage(QObject::tr("Saving aborted"), 2000); + return false; + } else if (ret == QMessageBox::Yes) { + return saveAs(); + } + } catch (const Base::Exception& e) { QMessageBox::critical(getMainWindow(), QObject::tr("Saving document failed"), QString::fromLatin1(e.what())); @@ -1942,11 +1980,33 @@ bool Document::canClose (bool checkModify, bool checkLink) bool ok = true; if (checkModify && isModified() && !getDocument()->testStatus(App::Document::PartialDoc)) { - int res = getMainWindow()->confirmSave(getDocument()->Label.getValue(),getActiveView()); - if(res>0) + const char *docName = getDocument()->Label.getValue(); + int res = getMainWindow()->confirmSave(docName, getActiveView()); + switch (res) + { + case MainWindow::ConfirmSaveResult::Cancel: + ok = false; + break; + case MainWindow::ConfirmSaveResult::SaveAll: + case MainWindow::ConfirmSaveResult::Save: ok = save(); - else - ok = res<0; + if (!ok) { + int ret = QMessageBox::question( + getActiveView(), + QObject::tr("Document not saved"), + QObject::tr("The document%1 could not be saved. Do you want to cancel closing it?") + .arg(docName?(QString::fromUtf8(" ")+QString::fromUtf8(docName)):QString()), + QMessageBox::Discard | QMessageBox::Cancel, + QMessageBox::Discard); + if (ret == QMessageBox::Discard) + ok = true; + } + break; + case MainWindow::ConfirmSaveResult::DiscardAll: + case MainWindow::ConfirmSaveResult::Discard: + ok = true; + break; + } } if (ok) { diff --git a/src/Gui/MainWindow.cpp b/src/Gui/MainWindow.cpp index 6d35aae1da..05801cbeb5 100644 --- a/src/Gui/MainWindow.cpp +++ b/src/Gui/MainWindow.cpp @@ -612,15 +612,15 @@ int MainWindow::confirmSave(const char *docName, QWidget *parent, bool addCheckb discardBtn->setShortcut(QKeySequence::mnemonic(text)); } - int res = 0; + int res = ConfirmSaveResult::Cancel; box.adjustSize(); // Silence warnings from Qt on Windows switch (box.exec()) { case QMessageBox::Save: - res = checkBox.isChecked()?2:1; + res = checkBox.isChecked()?ConfirmSaveResult::SaveAll:ConfirmSaveResult::Save; break; case QMessageBox::Discard: - res = checkBox.isChecked()?-2:-1; + res = checkBox.isChecked()?ConfirmSaveResult::DiscardAll:ConfirmSaveResult::Discard; break; } if(addCheckbox && res) @@ -632,12 +632,13 @@ bool MainWindow::closeAllDocuments (bool close) { auto docs = App::GetApplication().getDocuments(); try { - docs = App::Document::getDependentDocuments(docs,true); + docs = App::Document::getDependentDocuments(docs, true); }catch(Base::Exception &e) { e.ReportException(); } bool checkModify = true; bool saveAll = false; + int failedSaves = 0; for(auto doc : docs) { auto gdoc = Application::Instance->getDocument(doc); if(!gdoc) @@ -650,19 +651,36 @@ bool MainWindow::closeAllDocuments (bool close) continue; bool save = saveAll; if(!save && checkModify) { - int res = confirmSave(doc->Label.getStrValue().c_str(),this,docs.size()>1); - if(res==0) + int res = confirmSave(doc->Label.getStrValue().c_str(), this, docs.size()>1); + switch (res) + { + case ConfirmSaveResult::Cancel: return false; - if(res>0) { + case ConfirmSaveResult::SaveAll: + saveAll = true; + case ConfirmSaveResult::Save: save = true; - if(res==2) - saveAll = true; - } else if(res==-2) + break; + case ConfirmSaveResult::DiscardAll: checkModify = false; + } } + if(save && !gdoc->save()) + failedSaves++; + } + + if (failedSaves > 0) { + int ret = QMessageBox::question( + getMainWindow(), + QObject::tr("%1 Document(s) not saved").arg(QString::number(failedSaves)), + QObject::tr("Some documents could not be saved. Do you want to cancel closing?"), + QMessageBox::Discard | QMessageBox::Cancel, + QMessageBox::Discard); + if (ret == QMessageBox::Cancel) return false; } + if(close) App::GetApplication().closeAllDocuments(); // d->mdiArea->closeAllSubWindows(); diff --git a/src/Gui/MainWindow.h b/src/Gui/MainWindow.h index 3c864eeaf1..a980f0433c 100644 --- a/src/Gui/MainWindow.h +++ b/src/Gui/MainWindow.h @@ -77,6 +77,13 @@ class GuiExport MainWindow : public QMainWindow Q_OBJECT public: + enum ConfirmSaveResult { + Cancel = 0, + Save, + SaveAll, + Discard, + DiscardAll + }; /** * Constructs an empty main window. For default \a parent is 0, as there usually is * no toplevel window there.