On 11/21/2011 04:47 AM, Jean-Marc Lasgouttes wrote:
> Le 21/11/2011 03:20, Richard Heck a écrit :
>>> You have to call recordUndoFullDocument (should be
>>> recordUndoBufferParams, but alas this does not exist) on the master
>>> buffer, then. But I do not see where is the magic code that gives the
>>> master buffer.
>>>
>> I'll have a look at this. Before I do, is there any interaction between
>> this kind of thing and the whole ReadOnly issue? I.e., we have undo with
>> read-only documents?
>
> When a function which requires a buffer (== does not have the NoBuffer
> flag) does not have the ReadOnly flag, it is disabled as soon as the
> document is in read-only mode. In the old days, this was used as a
> signal for making a buffer dirty, but this has changed when fixing #3733.
> http://www.lyx.org/trac/ticket/3733
>
> Now, we set buffer dirty in reordUndo calls, which seems to make more
> sense.
>
> For branch activation, I would propose to first add a call to
> recordUndoFullDocument, which would be easier by moving the handler to
> BufferView.cpp (for now), until we sort out what to do with the undo
> helper. What we really need is a way to save the BufferParams only,
> not the buffer params plus the whole ParagraphList. The only problem
> is that this function requires a Cursor, which we do not have in
> Buffer.cpp.
>
So I investigated this a bit. There really isn't any way to do what
JMarc suggested in InsetBranch. The branch itself may (and usually will)
be held in the master, and that may not even be open in a view, so there
is no hope of getting a cursor so we can call recordUndoFullDocument()
on the correct document.
That said, it isn't obvious we should mark a document not open in a view
dirty. As things are, if this happens and you try to close LyX, you get:
Buffer.cpp(1240): /tmp/m.lyx.emergency
Warning: Attempting to close changed document!
----------------------------------------
LyX attempted to close a document that had unsaved changes!
LyX: Attempting to save document /tmp/m.lyx
Saved to /tmp/m.lyx.emergency. Phew.
LyX does not ask you if you want to save the document.
So I'm a little confused now about what to do. Maybe we simply shouldn't
try to mark the master dirty in this case.
Anyway, a patch is attached that deals with the Document>Settings issue,
but not with the context menu issue (which is handled at InsetBranch).
Comments?
Richard
>From b7d7810167f9dfc83dcbdffad4e429adb95b74bd Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Tue, 29 Nov 2011 15:38:57 -0500
Subject: [PATCH] Another shot at #7872.
Following suggestions of JMarc's, we use recordUndoFullDocument() to
mark the Buffer dirty. Morevoer, we do not try to mark the master
document dirty from InsetBranch, since this causes problems if the
master is not open in a view. E.g., if you try then to close LyX,
you are not asked if you want to save it, but LyX panics and does an
emergency save.
---
src/Buffer.cpp | 39 +------------------------------------
src/BufferView.cpp | 46 ++++++++++++++++++++++++++++++++++++++++++++
src/insets/InsetBranch.cpp | 5 +---
3 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index cd3dcb8..dea663e 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -2102,14 +2102,6 @@ bool Buffer::getStatus(FuncRequest const & cmd, FuncStatus & flag)
enable = params().isExportable("program");
break;
- case LFUN_BRANCH_ACTIVATE:
- case LFUN_BRANCH_DEACTIVATE: {
- BranchList const & branchList = params().branchlist();
- docstring const branchName = cmd.argument();
- enable = !branchName.empty() && branchList.find(branchName);
- break;
- }
-
case LFUN_BRANCH_ADD:
case LFUN_BRANCHES_RENAME:
case LFUN_BUFFER_PRINT:
@@ -2261,36 +2253,7 @@ void Buffer::dispatch(FuncRequest const & func, DispatchResult & dr)
break;
}
- case LFUN_BRANCH_ACTIVATE:
- case LFUN_BRANCH_DEACTIVATE: {
- BranchList & branchList = params().branchlist();
- docstring const branchName = func.argument();
- // the case without a branch name is handled elsewhere
- if (branchName.empty()) {
- dispatched = false;
- break;
- }
- Branch * branch = branchList.find(branchName);
- if (!branch) {
- LYXERR0("Branch " << branchName << " does not exist.");
- dr.setError(true);
- docstring const msg =
- bformat(_("Branch \"%1$s\" does not exist."), branchName);
- dr.setMessage(msg);
- break;
- }
- bool activate = func.action() == LFUN_BRANCH_ACTIVATE;
- if (branch->isSelected() != activate) {
- branch->setSelected(activate);
- markDirty();
- dr.setError(false);
- dr.screenUpdate(Update::Force);
- dr.forceBufferUpdate();
- }
- break;
- }
-
- case LFUN_BRANCHES_RENAME: {
+ case LFUN_BRANCHES_RENAME: {
if (func.argument().empty())
break;
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 178bcfc..2902e32 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -1160,6 +1160,17 @@ bool BufferView::getStatus(FuncRequest const & cmd, FuncStatus & flag)
break;
}
+ // FIXME We do not really want this here, but at present we need to
+ // handle their dispatch here, for reasons explained there, so we'll
+ // handle this here, too, for consistency.
+ case LFUN_BRANCH_ACTIVATE:
+ case LFUN_BRANCH_DEACTIVATE: {
+ BranchList const & branchList = buffer().params().branchlist();
+ docstring const branchName = cmd.argument();
+ flag.setEnabled(!branchName.empty() && branchList.find(branchName));
+ break;
+ }
+
default:
return false;
}
@@ -1909,6 +1920,41 @@ void BufferView::dispatch(FuncRequest const & cmd, DispatchResult & dr)
break;
}
+ // FIXME We do not really want this here, but it has to be at present
+ // because we need a cursor for the recordUndoFullDocument call. What
+ // we would really like is a recordUndoBufferParams call that did not
+ // need a cursor, but we do not have that yet.
+ // So, if this does get fixed, this code can be moved back to Buffer.cpp,
+ // and the corresponding code in getStatus() should be moved back, too.
+ case LFUN_BRANCH_ACTIVATE:
+ case LFUN_BRANCH_DEACTIVATE: {
+ BranchList & branch_list = buffer().params().branchlist();
+ docstring const branch_name = cmd.argument();
+ // the case without a branch name is handled elsewhere
+ if (branch_name.empty()) {
+ dispatched = false;
+ break;
+ }
+ Branch * branch = branch_list.find(branch_name);
+ if (!branch) {
+ LYXERR0("Branch " << branch_name << " does not exist.");
+ dr.setError(true);
+ docstring const msg =
+ bformat(_("Branch \"%1$s\" does not exist."), branch_name);
+ dr.setMessage(msg);
+ break;
+ }
+ bool activate = cmd.action() == LFUN_BRANCH_ACTIVATE;
+ if (branch->isSelected() != activate) {
+ branch->setSelected(activate);
+ cur.recordUndoFullDocument();
+ dr.setError(false);
+ dr.screenUpdate(Update::Force);
+ dr.forceBufferUpdate();
+ }
+ break;
+ }
+
default:
// OK, so try the Buffer itself...
buffer_.dispatch(cmd, dr);
diff --git a/src/insets/InsetBranch.cpp b/src/insets/InsetBranch.cpp
index ef40748..b6f250c 100644
--- a/src/insets/InsetBranch.cpp
+++ b/src/insets/InsetBranch.cpp
@@ -127,19 +127,16 @@ void InsetBranch::doDispatch(Cursor & cur, FuncRequest & cmd)
case LFUN_BRANCH_ACTIVATE:
case LFUN_BRANCH_DEACTIVATE: {
Buffer * buf = const_cast<Buffer *>(buffer().masterBuffer());
- BranchList & branchlist = buf->params().branchlist();
- Branch * our_branch = branchlist.find(params_.branch);
+ Branch * our_branch = buf->params().branchlist().find(params_.branch);
if (!our_branch) {
// child only?
our_branch = buffer().params().branchlist().find(params_.branch);
if (!our_branch)
break;
- buf = &buffer();
}
bool const activate = (cmd.action() == LFUN_BRANCH_ACTIVATE);
if (our_branch->isSelected() != activate) {
our_branch->setSelected(activate);
- buf->markDirty();
cur.forceBufferUpdate();
}
break;
--
1.7.4.4
