From: Jean-Marc Lasgouttes Date: Thu, 20 Jul 2023 21:42:34 +0000 (+0200) Subject: Improve branch activatiion LFUNs X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=35359a4c6f58a9bd660b7dd82c4be5638700d0d9;p=features.git 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 e439242502..e837a9b552 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(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(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 67826a3d5c..4905efb45d 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 c25f16a0c4..505871834f 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().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;