[LyX/master] Improve branch activatiion LFUNs
Jean-Marc Lasgouttes
lasgouttes at lyx.org
Sun Jul 23 15:55:31 UTC 2023
commit 35359a4c6f58a9bd660b7dd82c4be5638700d0d9
Author: Jean-Marc Lasgouttes <lasgouttes at lyx.org>
Date: Thu Jul 20 23:42:34 2023 +0200
Improve branch activatiion LFUNs
* put the code that is called both from Buffer and InsetBrach in the
two helper methods Buffer::branchActivationStatus() and
Buffer::branchActivationDispatch().
* Cleanup the code so that _MASTER_ lfuns are disabled when there is
no master document.
* When changing branches in the master buffer, make the buffer visible
if it is not, and make sure that undo information is recorded.
* The code in Buffer::dispatch is used first, and it gives control to
the branch inset code if no branch name has been specified.
Fixes bug #12588.
---
src/Buffer.cpp | 113 ++++++++++++++++++++++++++++++--------------
src/Buffer.h | 10 ++++-
src/insets/InsetBranch.cpp | 68 ++------------------------
3 files changed, 93 insertions(+), 98 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index e439242..e837a9b 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -2719,6 +2719,24 @@ void Buffer::markDepClean(string const & name)
}
+bool Buffer::branchActivationEnabled(FuncCode act, docstring const & branch) const
+{
+ bool const master = act == LFUN_BRANCH_MASTER_ACTIVATE
+ || act == LFUN_BRANCH_MASTER_DEACTIVATE;
+ bool const activate = act == LFUN_BRANCH_ACTIVATE
+ || act == LFUN_BRANCH_MASTER_ACTIVATE;
+ Buffer const * buf = master ? masterBuffer() : this;
+ Branch const * our_branch = buf->params().branchlist().find(branch);
+ // Can be disabled if
+ // - this is a _MASTER_ command and there is no master
+ // - the relevant buffer does not know the branch
+ // - the branch is already in the desired state
+ return ((!master || parent() != nullptr)
+ && !branch.empty() && our_branch
+ && our_branch->isSelected() != activate);
+}
+
+
bool Buffer::getStatus(FuncRequest const & cmd, FuncStatus & flag) const
{
if (isInternal()) {
@@ -2770,15 +2788,12 @@ bool Buffer::getStatus(FuncRequest const & cmd, FuncStatus & flag) const
case LFUN_BRANCH_ACTIVATE:
case LFUN_BRANCH_DEACTIVATE:
case LFUN_BRANCH_MASTER_ACTIVATE:
- case LFUN_BRANCH_MASTER_DEACTIVATE: {
- bool const master = (cmd.action() == LFUN_BRANCH_MASTER_ACTIVATE
- || cmd.action() == LFUN_BRANCH_MASTER_DEACTIVATE);
- BranchList const & branchList = master ? masterBuffer()->params().branchlist()
- : params().branchlist();
- docstring const & branchName = cmd.argument();
- flag.setEnabled(!branchName.empty() && branchList.find(branchName));
+ case LFUN_BRANCH_MASTER_DEACTIVATE:
+ // Let a branch inset handle that
+ if (cmd.argument().empty())
+ return false;
+ flag.setEnabled(branchActivationEnabled(cmd.action(), cmd.argument()));
break;
- }
case LFUN_BRANCH_ADD:
case LFUN_BRANCHES_RENAME:
@@ -2823,6 +2838,53 @@ bool Buffer::getStatus(FuncRequest const & cmd, FuncStatus & flag) const
}
+bool Buffer::branchActivationDispatch(FuncCode act, docstring const & branch)
+{
+ bool const master = (act == LFUN_BRANCH_MASTER_ACTIVATE
+ || act == LFUN_BRANCH_MASTER_DEACTIVATE);
+ bool const activate = (act == LFUN_BRANCH_ACTIVATE
+ || act == LFUN_BRANCH_MASTER_ACTIVATE);
+ Buffer * buf = master ? const_cast<Buffer *>(masterBuffer()) : this;
+ Branch * our_branch = buf->params().branchlist().find(branch);
+
+ // See comment in branchActivationStatus
+ if ((master && parent() == nullptr)
+ || !our_branch
+ || our_branch->isSelected() == activate)
+ return false;
+
+ if (master && !buf->hasGuiDelegate()
+ && (!theApp() || !theApp()->unhide(buf)))
+ // at least issue a warning for now (ugly, but better than dataloss).
+ frontend::Alert::warning(_("Branch state changes in master document"),
+ lyx::support::bformat(_("The state of the branch '%1$s' "
+ "was changed in the master file. "
+ "Please make sure to save the master."), branch), true);
+
+ UndoGroupHelper ugh(buf);
+ buf->undo().recordUndoBufferParams(CursorData());
+ our_branch->setSelected(activate);
+ // cur.forceBufferUpdate() is not enough)
+ buf->updateBuffer();
+
+ // if branch exists in a descendant, update previews.
+ // TODO: only needed if "Show preview" is enabled in the included inset.
+ bool exists_in_desc = false;
+ for (auto const & it : buf->getDescendants()) {
+ if (it->params().branchlist().find(branch))
+ exists_in_desc = true;
+ }
+ if (exists_in_desc) {
+ // TODO: ideally we would only update the previews of the
+ // specific children that have this branch directly or
+ // in one of their descendants
+ buf->removePreviews();
+ buf->updatePreviews();
+ }
+ return true;
+}
+
+
void Buffer::dispatch(string const & command, DispatchResult & result)
{
return dispatch(lyxaction.lookupFunc(command), result);
@@ -2834,6 +2896,7 @@ void Buffer::dispatch(string const & command, DispatchResult & result)
// whether we have a GUI or not. The boolean use_gui holds this information.
void Buffer::dispatch(FuncRequest const & func, DispatchResult & dr)
{
+ LYXERR(Debug::ACTION, "Buffer::dispatch: cmd: " << func);
if (isInternal()) {
// FIXME? if there is an Buffer LFUN that can be dispatched even
// if internal, put a switch '(cmd.action())' here.
@@ -2932,35 +2995,15 @@ void Buffer::dispatch(FuncRequest const & func, DispatchResult & dr)
case LFUN_BRANCH_DEACTIVATE:
case LFUN_BRANCH_MASTER_ACTIVATE:
case LFUN_BRANCH_MASTER_DEACTIVATE: {
- bool const master = (func.action() == LFUN_BRANCH_MASTER_ACTIVATE
- || func.action() == LFUN_BRANCH_MASTER_DEACTIVATE);
- Buffer * buf = master ? const_cast<Buffer *>(masterBuffer())
- : this;
-
- docstring const & branch_name = func.argument();
- // the case without a branch name is handled elsewhere
- if (branch_name.empty()) {
- dispatched = false;
- break;
- }
- Branch * branch = buf->params().branchlist().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;
+ // Let a branch inset handle that
+ if (func.argument().empty()) {
+ dr.dispatched(false);
+ return;
}
- bool const activate = (func.action() == LFUN_BRANCH_ACTIVATE
- || func.action() == LFUN_BRANCH_MASTER_ACTIVATE);
- if (branch->isSelected() != activate) {
- buf->undo().recordUndoBufferParams(CursorData());
- branch->setSelected(activate);
- dr.setError(false);
+ bool const res = branchActivationDispatch(func.action(), func.argument());
+ dr.setError(!res);
+ if (res)
dr.screenUpdate(Update::Force);
- dr.forceBufferUpdate();
- }
break;
}
diff --git a/src/Buffer.h b/src/Buffer.h
index 67826a3..4905efb 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -12,6 +12,7 @@
#ifndef BUFFER_H
#define BUFFER_H
+#include "FuncCode.h"
#include "OutputEnums.h"
#include "support/unique_ptr.h"
@@ -153,7 +154,7 @@ public:
/// Destructor
~Buffer();
- /// Clones the entire structure of which this Buffer is part,
+ /// Clones the entire structure of which this Buffer is part,
/// cloning all the children, too.
Buffer * cloneWithChildren() const;
/// Just clones this single Buffer. For autosave.
@@ -161,6 +162,10 @@ public:
///
bool isClone() const;
+ /// Helper method: dispatch this branch activation LFUN
+ /// Retunrs true if the function has been dispatched
+ bool branchActivationDispatch(FuncCode act, docstring const & branch);
+
/** High-level interface to buffer functionality.
This function parses a command string and executes it.
*/
@@ -169,6 +174,9 @@ public:
/// Maybe we know the function already by number...
void dispatch(FuncRequest const & func, DispatchResult & result);
+ /// Helper method: Is this buffer activation LFUN enabled?
+ bool branchActivationEnabled(FuncCode act, docstring const & branch) const;
+
/// Can this function be exectued?
/// \return true if we made a decision
bool getStatus(FuncRequest const & cmd, FuncStatus & flag) const;
diff --git a/src/insets/InsetBranch.cpp b/src/insets/InsetBranch.cpp
index c25f16a..5058718 100644
--- a/src/insets/InsetBranch.cpp
+++ b/src/insets/InsetBranch.cpp
@@ -166,54 +166,9 @@ void InsetBranch::doDispatch(Cursor & cur, FuncRequest & cmd)
case LFUN_BRANCH_ACTIVATE:
case LFUN_BRANCH_DEACTIVATE:
case LFUN_BRANCH_MASTER_ACTIVATE:
- case LFUN_BRANCH_MASTER_DEACTIVATE: {
- bool const master = (cmd.action() == LFUN_BRANCH_MASTER_ACTIVATE
- || cmd.action() == LFUN_BRANCH_MASTER_DEACTIVATE);
- Buffer * buf = master ? const_cast<Buffer *>(buffer().masterBuffer())
- : &buffer();
-
- Branch * our_branch = buf->params().branchlist().find(params_.branch);
- if (!our_branch)
- break;
-
- bool const activate = (cmd.action() == LFUN_BRANCH_ACTIVATE
- || cmd.action() == LFUN_BRANCH_MASTER_ACTIVATE);
- if (our_branch->isSelected() != activate) {
- // FIXME If the branch is in the master document, we cannot
- // call recordUndo..., because the master may be hidden, and
- // the code presently assumes that hidden documents can never
- // be dirty. See GuiView::closeBufferAll(), for example.
- // An option would be to check if the master is hidden.
- // If it is, unhide.
- if (!master)
- buffer().undo().recordUndoBufferParams(cur);
- else
- // at least issue a warning for now (ugly, but better than dataloss).
- frontend::Alert::warning(_("Branch state changes in master document"),
- lyx::support::bformat(_("The state of the branch '%1$s' "
- "was changed in the master file. "
- "Please make sure to save the master."), params_.branch), true);
- our_branch->setSelected(activate);
- // cur.forceBufferUpdate() is not enough
- buf->updateBuffer();
- }
-
- // if branch exists in a descendant, update previews.
- // TODO: only needed if "Show preview" is enabled in the included inset.
- bool exists_in_desc = false;
- for (auto const & it : buf->getDescendants()) {
- if (it->params().branchlist().find(params_.branch))
- exists_in_desc = true;
- }
- if (exists_in_desc) {
- // TODO: ideally we would only update the previews of the
- // specific children that have this branch directly or
- // in one of their descendants
- buf->removePreviews();
- buf->updatePreviews();
- }
+ case LFUN_BRANCH_MASTER_DEACTIVATE:
+ buffer().branchActivationDispatch(cmd.action(), params_.branch);
break;
- }
case LFUN_BRANCH_INVERT:
cur.recordUndoInset(this);
params_.inverted = !params_.inverted;
@@ -253,7 +208,10 @@ bool InsetBranch::getStatus(Cursor & cur, FuncRequest const & cmd,
break;
case LFUN_BRANCH_ACTIVATE:
- flag.setEnabled(known_branch && !isBranchSelected(true));
+ case LFUN_BRANCH_DEACTIVATE:
+ case LFUN_BRANCH_MASTER_ACTIVATE:
+ case LFUN_BRANCH_MASTER_DEACTIVATE:
+ flag.setEnabled(buffer().branchActivationEnabled(cmd.action(), params_.branch));
break;
case LFUN_BRANCH_INVERT:
@@ -265,20 +223,6 @@ bool InsetBranch::getStatus(Cursor & cur, FuncRequest const & cmd,
flag.setEnabled(!known_branch);
break;
- case LFUN_BRANCH_DEACTIVATE:
- flag.setEnabled(isBranchSelected(true));
- break;
-
- case LFUN_BRANCH_MASTER_ACTIVATE:
- flag.setEnabled(buffer().parent()
- && buffer().masterBuffer()->params().branchlist().find(params_.branch)
- && !isBranchSelected());
- break;
-
- case LFUN_BRANCH_MASTER_DEACTIVATE:
- flag.setEnabled(buffer().parent() && isBranchSelected());
- break;
-
case LFUN_BRANCH_SYNC_ALL:
flag.setEnabled(known_branch);
break;
More information about the lyx-cvs
mailing list