On 08/11/2009 08:56 AM, Jürgen Spitzmüller wrote:
#5893  LyX closes hidden dirty buffers without asking
=>  I have played a bit with this, but did not come up with a satisfactory
solution. I'm out of time ATM, so if no one else picks this up, we will have
to live with it (I know it's a dataloss, but we have this since 1.6.0).

I've looked into this a bit, and I think the problem is that GuiView::closeBufferAll() is work area-oriented. It really iterates over the various document views, and closes them. In many cases, it explicitly does not close the Buffer at all, and it certainly doesn't close any parent buffers. These are left to be destroyed on exit. In any event, it seems not to consider hidden buffers at all.

I understand why this happens, I think, namely that this means to be destroying only a *window*. We wouldn't e.g. want to close a buffer that might be open in some other view.

I propose something along the following lines, but I am not at all sure about this. I'm trying to do two things here. (i) Check whether a Buffer is dirty whenever it is destroyed. I see this as a last ditch effort to avoid dataloss, and one that should rarely if ever be triggered. Indeed, in some ways I'm inclined to assert here...after saving! (ii) Do the same check whenever a tab is hidden. I don't think dirty tabs should be allowed to be hidden. It will lead to surprises later.

Comments welcome. I'm not at all sure about this.

rh

Index: src/Buffer.cpp
===================================================================
--- src/Buffer.cpp	(revision 30978)
+++ src/Buffer.cpp	(working copy)
@@ -328,6 +328,20 @@
 			theBufferList().releaseChild(this, child);
 	}
 
+	if (!isClean()) {
+		docstring const text = bformat(_("The document %1$s has unsaved changes."
+			"\n\nDo you want to save the document or discard the changes?"), from_utf8(absFileName()));
+		int const ret = Alert::prompt(_("Save changed document?"),
+		                text, 0, 2, _("&Save"), _("&Discard"));
+		switch (ret) {
+		case 0:
+			save();
+			break;
+		case 1:
+			break;
+		}
+	}
+
 	// clear references to children in macro tables
 	d->children_positions.clear();
 	d->position_to_children.clear();
Index: src/frontends/qt4/GuiView.cpp
===================================================================
--- src/frontends/qt4/GuiView.cpp	(revision 30978)
+++ src/frontends/qt4/GuiView.cpp	(working copy)
@@ -1929,27 +1929,7 @@
 }
 
 
-bool GuiView::closeBuffer(Buffer & buf, bool tolastopened, bool mark_active)
-{
-	// goto bookmark to update bookmark pit.
-	//FIXME: we should update only the bookmarks related to this buffer!
-	LYXERR(Debug::DEBUG, "GuiView::closeBuffer()");
-	for (size_t i = 0; i < theSession().bookmarks().size(); ++i)
-		theLyXFunc().gotoBookmark(i+1, false, false);
-
-	if (buf.isClean() || buf.paragraphs().empty()) {
-		// save in sessions if requested
-		// do not save childs if their master
-		// is opened as well
-		if (tolastopened)
-			theSession().lastOpened().add(buf.fileName(), mark_active);
-		if (buf.parent())
-			// Don't close child documents.
-			removeWorkArea(currentMainWorkArea());
-		else
-			theBufferList().release(&buf);
-		return true;
-	}
+bool GuiView::saveIfDirty(Buffer & buf) {
 	// Switch to this Buffer.
 	setBuffer(&buf);
 
@@ -1983,7 +1963,35 @@
 	case 2:
 		return false;
 	}
+	return true;
+}
 
+
+bool GuiView::closeBuffer(Buffer & buf, bool tolastopened, bool mark_active)
+{
+	// goto bookmark to update bookmark pit.
+	//FIXME: we should update only the bookmarks related to this buffer!
+	LYXERR(Debug::DEBUG, "GuiView::closeBuffer()");
+	for (size_t i = 0; i < theSession().bookmarks().size(); ++i)
+		theLyXFunc().gotoBookmark(i+1, false, false);
+
+	if (buf.isClean() || buf.paragraphs().empty()) {
+		// save in sessions if requested
+		// do not save childs if their master
+		// is opened as well
+		if (tolastopened)
+			theSession().lastOpened().add(buf.fileName(), mark_active);
+		if (buf.parent())
+			// Don't close child documents.
+			removeWorkArea(currentMainWorkArea());
+		else
+			theBufferList().release(&buf);
+		return true;
+	}
+	
+	if (!saveIfDirty(buf))
+		return false;
+
 	// save file names to .lyx/session
 	if (tolastopened)
 		theSession().lastOpened().add(buf.fileName(), mark_active);
Index: src/frontends/qt4/GuiView.h
===================================================================
--- src/frontends/qt4/GuiView.h	(revision 30978)
+++ src/frontends/qt4/GuiView.h	(working copy)
@@ -253,6 +253,8 @@
 
 	///
 	void updateCompletion(Cursor & cur, bool start, bool keep);
+	///
+	bool saveIfDirty(Buffer & buf);	
 
 private:
 	///
Index: src/frontends/qt4/GuiWorkArea.cpp
===================================================================
--- src/frontends/qt4/GuiWorkArea.cpp	(revision 30978)
+++ src/frontends/qt4/GuiWorkArea.cpp	(working copy)
@@ -1475,6 +1475,12 @@
 	if (index == -1)
 		return false;
 
+	Buffer & buf = work_area->bufferView().buffer();
+	if (!buf.isClean()) {
+		if (!work_area->view().saveIfDirty(buf))
+			return false;
+	}
+	
 	work_area->setUpdatesEnabled(false);
 	removeTab(index);
 	delete work_area;

Reply via email to