From c9f7ce0a7a6d48b9b60af92828654aa7547286b4 Mon Sep 17 00:00:00 2001 From: Guillaume Munch Date: Fri, 11 Dec 2015 02:15:52 +0000 Subject: [PATCH] New LFUN tabular-feature (#9794) The tabular-features LFUN was merged with "inset-modify tabular" when simplifying the tabular dialog at b5049e7. This choice later indirectly caused a few regressions (#7308, #9794). I reintroduce tabular-feature to allow more flexibility for user commands, whereas "inset-modify tabular" is now reserved for the tabular dialog. In particular, inset-modify tabular is no longer caught by math grid insets. The name tabular-feature is kept to avoid renaming icons. Known issues: * After successfully applying a tabular command, the cursor is truncated to the table. * Note that the tabular dialog still has similar issues that are inherited from the achitecture of the dialog menu. For instance the pref change can be mis-dispatched to an inset inside a cell and cause an error, for instance: Lexer.cpp (934): Missing 'Note'-tag in InsetNote::string2params. Got tabular instead. Line: 0 Maybe the inset-modify LFUN should be modified to treat commands coming from the wrong dialog (by checking the type) as unknown and undispatched so that the parent can get it. In that case a non AtPoint variant of inset-modify could be reintroduced in order to generalise tabular-feature. See: http://mid.gmane.org/n4rdk1$efj$1@ger.gmane.org --- src/FuncCode.h | 1 + src/LyXAction.cpp | 25 ++++++--- src/frontends/qt4/GuiApplication.cpp | 11 ++-- src/frontends/qt4/GuiTabular.cpp | 7 +-- src/insets/Inset.cpp | 2 - src/insets/InsetTabular.cpp | 83 ++++++++++++++++------------ src/insets/InsetTabular.h | 5 +- src/mathed/InsetMathAMSArray.cpp | 9 +-- src/mathed/InsetMathCases.cpp | 20 ++----- src/mathed/InsetMathGrid.cpp | 25 +++------ src/mathed/InsetMathHull.cpp | 9 +-- src/mathed/InsetMathSplit.cpp | 12 ++-- src/mathed/InsetMathSubstack.cpp | 16 ++---- 13 files changed, 103 insertions(+), 122 deletions(-) diff --git a/src/FuncCode.h b/src/FuncCode.h index 66f57c0107..40ec9d3543 100644 --- a/src/FuncCode.h +++ b/src/FuncCode.h @@ -464,6 +464,7 @@ enum FuncCode LFUN_BUFFER_MOVE_NEXT, // skostysh 20150408 // 340 LFUN_BUFFER_MOVE_PREVIOUS, // skostysh 20150408 + LFUN_TABULAR_FEATURE, // gm, 20151210 LFUN_LASTACTION // end of the table }; diff --git a/src/LyXAction.cpp b/src/LyXAction.cpp index 44672193cb..1e8390575a 100644 --- a/src/LyXAction.cpp +++ b/src/LyXAction.cpp @@ -2167,7 +2167,7 @@ void LyXAction::init() /*! * \var lyx::FuncCode lyx::LFUN_TABULAR_INSERT * \li Action: Inserts table into the document. - * \li Notion: See #LFUN_INSET_MODIFY for some more details + * \li Notion: See #LFUN_TABULAR_FEATURE for some more details about tabular modifications. * \li Syntax: tabular-insert [ ] * \li Params: In case no arguments are given show insert dialog. @@ -2462,11 +2462,17 @@ void LyXAction::init() ref, space, tabular, vspace, wrap insets. * \li Syntax: inset-modify * \li Syntax: inset-modify changetype - * \li Syntax: inset-modify tabular [] + * \li Sample: inset-modify note Note Comment \n + inset-modify changetype Ovalbox + * \endvar + */ + { LFUN_INSET_MODIFY, "inset-modify", AtPoint, Edit }, +/*! + * \var lyx::FuncCode lyx::LFUN_TABULAR_FEATURE + * \li Action: Modify properties of tabulars and table-like math environments. + * \li Syntax: tabular-feature [] * \li Params: Generally see #LFUN_INSET_INSERT for further details.\n - In case that is "tabular" various math-environment features - are handled as well, e.g. add-vline-left/right for the Grid/Array environment.\n - : append-row|append-column|delete-row|delete-column|copy-row|\n + * : append-row|append-column|delete-row|delete-column|copy-row|\n copy-column|move-column-right|move-column-left|move-row-down|move-row-up|\n toggle-line-top|toggle-line-bottom|toggle-line-left|toggle-line-right|\n align-left|align-right|align-center|align-block|align-decimal|set-decimal-point|\n @@ -2481,13 +2487,14 @@ void LyXAction::init() set-special-column|set-special-multicolumn|set-special-multirow|\n toggle-booktabs|set-booktabs|unset-booktabs|set-top-space|set-bottom-space|\n set-interline-space|set-border-lines|tabular-valign-top|\n - tabular-valign-middle|tabular-valign-bottom|set-tabular-width + tabular-valign-middle|tabular-valign-bottom|set-tabular-width\n + Various math-environment features are handled as well, e.g. add-vline-left/right for\n + the Grid/Array environment.\n : additional argument for some commands, use debug mode to explore its values. - * \li Sample: inset-modify note Note Comment \n - inset-modify changetype Ovalbox + * \li Origin: gm, 10 Dec 2015 * \endvar */ - { LFUN_INSET_MODIFY, "inset-modify", AtPoint, Edit }, + { LFUN_TABULAR_FEATURE, "tabular-feature", Noop, Edit }, /*! * \var lyx::FuncCode lyx::LFUN_INSET_DIALOG_UPDATE * \li Action: Updates the values inside the dialog from the inset. diff --git a/src/frontends/qt4/GuiApplication.cpp b/src/frontends/qt4/GuiApplication.cpp index 4b6e769ddf..82bbd60d46 100644 --- a/src/frontends/qt4/GuiApplication.cpp +++ b/src/frontends/qt4/GuiApplication.cpp @@ -486,19 +486,18 @@ QString iconName(FuncRequest const & f, bool unknown) docstring firstcom; docstring dummy = split(f.argument(), firstcom, ';'); name1 = toqstr(firstcom); - // FIXME: we should rename the icons to tabular-xxx instead of - // "tabular-feature-xxx" + // FIXME: we should remove all references to "inset-modify tabular" + // icons and shortcuts in all manuals name1.replace("inset-modify tabular", "tabular-feature"); name1.replace(' ', '_'); break; } case LFUN_INSET_MODIFY: { - // FIXME: we should rename the icons to tabular-xxx instead of - // "tabular-feature-xxx" and generalize this naming to all - // insets, not to tabular using ones. + // FIXME: we should remove all references to "inset-modify tabular" + // icons and shortcuts in all manuals string inset_name; string const command = split(to_utf8(f.argument()), inset_name, ' '); - if (insetCode(inset_name) == TABULAR_CODE) { + if (inset_name == "tabular") { name1 = "tabular-feature "+ toqstr(command); name1.replace(' ', '_'); break; diff --git a/src/frontends/qt4/GuiTabular.cpp b/src/frontends/qt4/GuiTabular.cpp index 495b2c6277..fde382b936 100644 --- a/src/frontends/qt4/GuiTabular.cpp +++ b/src/frontends/qt4/GuiTabular.cpp @@ -447,8 +447,7 @@ void GuiTabular::setTableAlignment(string & param_str) const docstring GuiTabular::dialogToParams() const { - // FIXME: We should use Tabular directly. - string param_str = "tabular from-dialog"; + string param_str = "tabular"; // table width string tabwidth = widgetsToLength(tabularWidthED, tabularWidthUnitLC); @@ -1070,8 +1069,8 @@ bool GuiTabular::checkWidgets(bool readonly) const bool GuiTabular::funcEnabled(Tabular::Feature f) const { - string cmd = "tabular " + featureAsString(f); - return getStatus(FuncRequest(LFUN_INSET_MODIFY, cmd)).enabled(); + FuncRequest r(LFUN_INSET_MODIFY, "tabular for-dialog" + featureAsString(f)); + return getStatus(r).enabled(); } diff --git a/src/insets/Inset.cpp b/src/insets/Inset.cpp index 0fce648eff..f85b1a9566 100644 --- a/src/insets/Inset.cpp +++ b/src/insets/Inset.cpp @@ -391,8 +391,6 @@ bool Inset::getStatus(Cursor &, FuncRequest const & cmd, // FIXME: Why don't we let the insets determine whether this // should be enabled or not ? Now we need this check for // the tabular features. (vfr) - if (cmd.getArg(0) == "tabular") - return false; flag.setEnabled(true); return true; diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp index 437ac7eca7..3aeac0a9e8 100644 --- a/src/insets/InsetTabular.cpp +++ b/src/insets/InsetTabular.cpp @@ -110,6 +110,8 @@ TabularFeature tabularFeature[] = { // the SET/UNSET actions are used by the table dialog, // the TOGGLE actions by the table toolbar buttons + // FIXME: these values have been hardcoded in InsetMathGrid and other + // math insets. { Tabular::APPEND_ROW, "append-row", false }, { Tabular::APPEND_COLUMN, "append-column", false }, { Tabular::DELETE_ROW, "delete-row", false }, @@ -4298,16 +4300,17 @@ void InsetTabular::doDispatch(Cursor & cur, FuncRequest & cmd) cur.bv().showDialog("tabular"); break; - case LFUN_INSET_MODIFY: { - string arg; - if (cmd.getArg(1) == "from-dialog") - arg = cmd.getArg(0) + to_utf8(cmd.argument().substr(19)); + case LFUN_INSET_MODIFY: + // we come from the dialog + if (cmd.getArg(0) == "tabular") + tabularFeatures(cur, cmd.getLongArg(1)); else - arg = to_utf8(cmd.argument()); - if (!tabularFeatures(cur, arg)) cur.undispatched(); break; - } + + case LFUN_TABULAR_FEATURE: + tabularFeatures(cur, to_utf8(cmd.argument())); + break; // insert file functions case LFUN_FILE_INSERT_PLAINTEXT_PARA: @@ -4474,25 +4477,9 @@ void InsetTabular::doDispatch(Cursor & cur, FuncRequest & cmd) } -// function sets an object as defined in func_status.h: -// states OK, Unknown, Disabled, On, Off. -bool InsetTabular::getStatus(Cursor & cur, FuncRequest const & cmd, - FuncStatus & status) const +bool InsetTabular::getFeatureStatus(Cursor & cur, string const & s, + string const & argument, FuncStatus & status) const { - switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - if (&cur.inset() != this || cmd.getArg(0) != "tabular") - break; - - // FIXME: We only check for the very first argument... - string const s = cmd.getArg(1); - // We always enable the lfun if it is coming from the dialog - // because the dialog makes sure all the settings are valid, - // even though the first argument might not be valid now. - if (s == "from-dialog") { - status.setEnabled(true); - return true; - } int action = Tabular::LAST_ACTION; int i = 0; @@ -4508,8 +4495,6 @@ bool InsetTabular::getStatus(Cursor & cur, FuncRequest const & cmd, return true; } - string const argument = cmd.getLongArg(2); - row_type sel_row_start = 0; row_type sel_row_end = 0; col_type sel_col_start = 0; @@ -4884,6 +4869,39 @@ bool InsetTabular::getStatus(Cursor & cur, FuncRequest const & cmd, break; } return true; +} + + +// function sets an object as defined in func_status.h: +// states OK, Unknown, Disabled, On, Off. +bool InsetTabular::getStatus(Cursor & cur, FuncRequest const & cmd, + FuncStatus & status) const +{ + switch (cmd.action()) { + case LFUN_INSET_MODIFY: + if (cmd.getArg(0) != "tabular") + break; + if (cmd.getArg(1) == "for-dialog") { + // The dialog is asking the status of a command + if (&cur.inset() != this) + break; + string action = cmd.getArg(2); + string arg = cmd.getLongArg(3); + return getFeatureStatus(cur, action, arg, status); + } else { + // We always enable the lfun if it is coming from the dialog + // because the dialog makes sure all the settings are valid, + // even though the first argument might not be valid now. + status.setEnabled(true); + return true; + } + + case LFUN_TABULAR_FEATURE: { + if (&cur.inset() != this) + break; + string action = cmd.getArg(0); + string arg = cmd.getLongArg(1); + return getFeatureStatus(cur, action, arg, status); } case LFUN_CAPTION_INSERT: { @@ -5311,23 +5329,19 @@ void InsetTabular::movePrevCell(Cursor & cur, EntryDirection entry_from) } -bool InsetTabular::tabularFeatures(Cursor & cur, string const & argument) +void InsetTabular::tabularFeatures(Cursor & cur, string const & argument) { istringstream is(argument); string s; - is >> s; - if (insetCode(s) != TABULAR_CODE) - return false; - // Safe guard. size_t safe_guard = 0; for (;;) { if (is.eof()) - break; + return; safe_guard++; if (safe_guard > 1000) { LYXERR0("parameter max count reached!"); - break; + return; } is >> s; Tabular::Feature action = Tabular::LAST_ACTION; @@ -5349,7 +5363,6 @@ bool InsetTabular::tabularFeatures(Cursor & cur, string const & argument) LYXERR(Debug::DEBUG, "Feature: " << s << "\t\tvalue: " << val); tabularFeatures(cur, action, val); } - return true; } diff --git a/src/insets/InsetTabular.h b/src/insets/InsetTabular.h index d9b7fa442c..026489d155 100644 --- a/src/insets/InsetTabular.h +++ b/src/insets/InsetTabular.h @@ -904,7 +904,7 @@ public: void cursorPos(BufferView const & bv, CursorSlice const & sl, bool boundary, int & x, int & y) const; /// - bool tabularFeatures(Cursor & cur, std::string const & what); + void tabularFeatures(Cursor & cur, std::string const & what); /// void tabularFeatures(Cursor & cur, Tabular::Feature feature, std::string const & val = std::string()); @@ -996,6 +996,9 @@ private: /// void doDispatch(Cursor & cur, FuncRequest & cmd); /// + bool getFeatureStatus(Cursor & cur, std::string const & s, + std::string const & argument, FuncStatus & status) const; + /// bool getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus &) const; /// Inset * clone() const { return new InsetTabular(*this); } diff --git a/src/mathed/InsetMathAMSArray.cpp b/src/mathed/InsetMathAMSArray.cpp index 12a92da080..13d7a9425e 100644 --- a/src/mathed/InsetMathAMSArray.cpp +++ b/src/mathed/InsetMathAMSArray.cpp @@ -107,13 +107,8 @@ bool InsetMathAMSArray::getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus & flag) const { switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") - break; - is >> s; + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); if (s == "add-vline-left" || s == "add-vline-right") { flag.message(bformat( from_utf8(N_("Can't add vertical grid lines in '%1$s'")), diff --git a/src/mathed/InsetMathCases.cpp b/src/mathed/InsetMathCases.cpp index 17b4fb1469..1b5df8dc9f 100644 --- a/src/mathed/InsetMathCases.cpp +++ b/src/mathed/InsetMathCases.cpp @@ -71,14 +71,11 @@ void InsetMathCases::doDispatch(Cursor & cur, FuncRequest & cmd) { //lyxerr << "*** InsetMathCases: request: " << cmd << endl; switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") - break; - is >> s; + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); // vertical lines and adding/deleting columns is not allowed for \cases + // FIXME: "I suspect that the break after cur.undispatched() should be a + // return; the recordUndo seems bogus too." (lasgouttes) if (s == "append-column" || s == "delete-column" || s == "add-vline-left" || s == "add-vline-right") { cur.undispatched(); @@ -97,13 +94,8 @@ bool InsetMathCases::getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus & flag) const { switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") - break; - is >> s; + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); if (s == "add-vline-left" || s == "add-vline-right") { flag.setEnabled(false); flag.message(bformat( diff --git a/src/mathed/InsetMathGrid.cpp b/src/mathed/InsetMathGrid.cpp index 536f4bd163..12e871aa5c 100644 --- a/src/mathed/InsetMathGrid.cpp +++ b/src/mathed/InsetMathGrid.cpp @@ -181,7 +181,7 @@ void InsetMathGrid::setDefaults() bool InsetMathGrid::interpretString(Cursor & cur, docstring const & str) { if (str == "\\hline") { - FuncRequest fr = FuncRequest(LFUN_INSET_MODIFY, "tabular add-hline-above"); + FuncRequest fr = FuncRequest(LFUN_TABULAR_FEATURE, "add-hline-above"); FuncStatus status; if (getStatus(cur, fr, status)) { if (status.enabled()) { @@ -1450,17 +1450,12 @@ void InsetMathGrid::doDispatch(Cursor & cur, FuncRequest & cmd) break; } - case LFUN_INSET_MODIFY: { + case LFUN_TABULAR_FEATURE: { cur.recordUndoInset(); //lyxerr << "handling tabular-feature " << to_utf8(cmd.argument()) << endl; istringstream is(to_utf8(cmd.argument())); string s; is >> s; - if (s != "tabular") { - InsetMathNest::doDispatch(cur, cmd); - return; - } - is >> s; if (s == "valign-top") setVerticalAlignment('t'); else if (s == "valign-middle") @@ -1597,7 +1592,7 @@ void InsetMathGrid::doDispatch(Cursor & cur, FuncRequest & cmd) } bool hline_enabled = false; - FuncRequest fr = FuncRequest(LFUN_INSET_MODIFY, "tabular add-hline-above"); + FuncRequest fr = FuncRequest(LFUN_TABULAR_FEATURE, "add-hline-above"); FuncStatus status; if (getStatus(cur, fr, status)) hline_enabled = status.enabled(); @@ -1727,14 +1722,8 @@ bool InsetMathGrid::getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus & status) const { switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") { - // We only now about table actions here. - break; - } + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); if (&cur.inset() != this) { // Table actions requires that the cursor is _inside_ the // table. @@ -1742,7 +1731,6 @@ bool InsetMathGrid::getStatus(Cursor & cur, FuncRequest const & cmd, status.message(from_utf8(N_("Cursor not in table"))); return true; } - is >> s; if (nrows() <= 1 && (s == "delete-row" || s == "swap-row")) { status.setEnabled(false); status.message(from_utf8(N_("Only one row"))); @@ -1797,7 +1785,8 @@ bool InsetMathGrid::getStatus(Cursor & cur, FuncRequest const & cmd, } else { status.setEnabled(false); status.message(bformat( - from_utf8(N_("Unknown tabular feature '%1$s'")), lyx::from_ascii(s))); + from_utf8(N_("Unknown tabular feature '%1$s'")), + from_utf8(s))); } #if 0 diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp index 097a3445c5..5b884b4a62 100644 --- a/src/mathed/InsetMathHull.cpp +++ b/src/mathed/InsetMathHull.cpp @@ -1816,13 +1816,8 @@ bool InsetMathHull::getStatus(Cursor & cur, FuncRequest const & cmd, } return InsetMathGrid::getStatus(cur, cmd, status); - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") - return InsetMathGrid::getStatus(cur, cmd, status); - is >> s; + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); if (!rowChangeOK() && (s == "append-row" || s == "delete-row" diff --git a/src/mathed/InsetMathSplit.cpp b/src/mathed/InsetMathSplit.cpp index 5c425fb002..2adb84522b 100644 --- a/src/mathed/InsetMathSplit.cpp +++ b/src/mathed/InsetMathSplit.cpp @@ -73,16 +73,12 @@ bool InsetMathSplit::getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus & flag) const { switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") - break; - is >> s; + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); if (s == "add-vline-left" || s == "add-vline-right") { flag.message(bformat( - from_utf8(N_("Can't add vertical grid lines in '%1$s'")), name_)); + from_utf8(N_("Can't add vertical grid lines in '%1$s'")), + name_)); flag.setEnabled(false); return true; } diff --git a/src/mathed/InsetMathSubstack.cpp b/src/mathed/InsetMathSubstack.cpp index 7810ff479e..f88f4927e3 100644 --- a/src/mathed/InsetMathSubstack.cpp +++ b/src/mathed/InsetMathSubstack.cpp @@ -63,27 +63,21 @@ bool InsetMathSubstack::getStatus(Cursor & cur, FuncRequest const & cmd, FuncStatus & flag) const { switch (cmd.action()) { - case LFUN_INSET_MODIFY: { - istringstream is(to_utf8(cmd.argument())); - string s; - is >> s; - if (s != "tabular") - break; - is >> s; - string const name = "substack"; + case LFUN_TABULAR_FEATURE: { + string s = cmd.getArg(0); if (s == "add-vline-left" || s == "add-vline-right") { flag.message(bformat( from_utf8(N_("Can't add vertical grid lines in '%1$s'")), - from_utf8(name))); + from_utf8("substack"))); flag.setEnabled(false); return true; } - // in contrary to \subaray, the columns in \substack + // in contrary to \subarray, the columns in \substack // are always centered and this cannot be changed if (s == "align-left" || s == "align-right") { flag.message(bformat( from_utf8(N_("Can't change horizontal alignment in '%1$s'")), - from_utf8(name))); + from_utf8("substack"))); flag.setEnabled(false); return true; }