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.

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index e4392425024ebefb4d6baed419156bd01fd10568..e837a9b55284ed282d5dcd51620c62c20a2f46b9 100644 (file)
--- 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 67826a3d5ccc211ad3f87179a303475001e74422..4905efb45d03ddc1dec8e8f424addf8596af8006 100644 (file)
--- 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 c25f16a0c4c66d3f49d16a09021ced9bf12f8f3b..505871834fa5c1615360269768a76f71117edff8 100644 (file)
--- 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;