This patch addresses the remaining issue with crashes on ill-formed
inset-insert minibuffer commands. I've also added some comments
expressing my understanding of what's going on here.
Comments welcome. Testing desired.
Richard
--
==================================================================
Richard G Heck, Jr
Professor of Philosophy
Brown University
http://frege.brown.edu/heck/
==================================================================
Get my public key from http://sks.keyserver.penguin.de
Hash: 0x1DE91F1E66FFBDEC
Learn how to sign your email using Thunderbird and GnuPG at:
http://dudu.dyn.2-h.org/nist/gpg-enigmail-howto
Index: insetcommandparams.C
===================================================================
--- insetcommandparams.C (revision 17850)
+++ insetcommandparams.C (working copy)
@@ -35,20 +35,39 @@
using support::WarningException;
InsetCommandParams::InsetCommandParams(string const & name)
- : name_(name), preview_(false)
+ : name_(name), type_(command2ParamType(name)), preview_(false)
{
info_ = findInfo(name);
BOOST_ASSERT(info_);
params_.resize(info_->n);
}
+//NOTE To add a new command to this file:
+// (i) If this represents a new inset type, add a new value to the
+// InsetParamType enum in insetcommandparams.h and add an
+// appropriate branch to command2ParamType. If not, then expand
+// the relevant branch there.
+// (ii) Add the relevant information to findInfo(). The possible parameters
+// should be specified in the order in which they should be output for
+// LaTeX. The isoptional[] array specifies whether the parameter is
+// optional in the sense of LaTeX. So, for example, getCommand() for
+// bibtex will output:
+// \bibtex[options][btprint]{bibfiles}
+// with the optional arguments printed only when necessary.
+// No parameter may be named "preview", because that is a required
+// flag for all commands.
+//It might seem obvious to use command2ParamType here and do a
+//check on that instead of on name. That, however, would assume that
+//the list of parameters does not depend up the command associated
+//with the inset. At present (4/07), that is true, but it could change.
+//If not, then perhaps this change should be made. That would simplify
+//findInfo() and allow removal of the complex code in setCmdName().
+
+//See the discussion in http://bugzilla.lyx.org/show_bug.cgi?id=3463.
InsetCommandParams::CommandInfo const *
InsetCommandParams::findInfo(std::string const & name)
{
- // No parameter may be named "preview", because that is a required
- // flag for all commands.
-
// InsetBibitem
if (name == "bibitem") {
static const char * const paramnames[] = {"label", "key", ""};
@@ -164,17 +183,115 @@
return &info;
}
- return 0;
+ lyxerr << "InsetCommandParams: Unknown inset type." << endl;
+ throw ExceptionMessage(WarningException,
+ _("Unknown inset type"),
+ from_utf8("Unknown inset type"));
}
+InsetCommandParams::InsetParamType InsetCommandParams::command2ParamType(std::string const & cmd)
+{
+ // InsetBibitem
+ if (cmd == "bibitem")
+ return ICP_BIBITEM;
+
+ // InsetBibtex
+ if (cmd == "bibtex")
+ return ICP_BIBTEX;
+
+ // InsetCitation
+ // FIXME: Use is_possible_cite_command() in
+ // src/frontends/controllers/biblio.C, see comment in src/factory.C.
+ if (cmd == "cite" || cmd == "citet" || cmd == "citep" || cmd == "citealt" ||
+ cmd == "citealp" || cmd == "citeauthor" || cmd == "citeyear" ||
+ cmd == "citeyearpar" || cmd == "citet*" || cmd == "citep*" ||
+ cmd == "citealt*" || cmd == "citealp*" ||
+ cmd == "citeauthor*" || cmd == "Citet" || cmd == "Citep" ||
+ cmd == "Citealt" || cmd == "Citealp" || cmd == "Citeauthor" ||
+ cmd == "Citet*" || cmd == "Citep*" || cmd == "Citealt*" ||
+ cmd == "Citealp*" || cmd == "Citeauthor*" ||
+ cmd == "citefield" || cmd == "citetitle" || cmd == "cite*")
+ return ICP_CITATION;
+
+ // InsetFloatlist
+ if (cmd == "floatlist")
+ return ICP_FLOATLIST;
+
+ // InsetHfill
+ if (cmd == "hfill")
+ return ICP_HFILL;
+
+ // InsetInclude
+ if (cmd == "include" || cmd == "input" || cmd == "verbatiminput" ||
+ cmd == "verbatiminput*")
+ return ICP_INCLUDE;
+
+ // InsetIndex
+ if (cmd == "index")
+ return ICP_INDEX;
+
+ //InsetPrintIndex
+ if (cmd == "printindex")
+ return ICP_PRINTINDEX;
+
+ //InsetLabel
+ if (cmd == "label")
+ return ICP_LABEL;
+
+ // InsetNomencl
+ if (cmd == "nomenclature")
+ return ICP_NOMENCL;
+
+ // InsetPrintNomencl
+ if (cmd == "printnomenclature")
+ return ICP_PRINTNOMENCL;
+
+ // InsetRef
+ if (cmd == "eqref" || cmd == "pageref" || cmd == "vpageref" ||
+ cmd == "vref" || cmd == "prettyref" || cmd == "ref")
+ return ICP_REF;
+
+ // InsetTOC
+ if (cmd == "tableofcontents")
+ return ICP_TOC;
+
+ // InsetUrl
+ if (cmd == "htmlurl" || cmd == "url")
+ return ICP_URL;
+
+ lyxerr << "InsetCommandParams: Unknown inset name: " << cmd << endl;
+ throw ExceptionMessage(WarningException,
+ _("Unknown inset command"),
+ from_utf8(cmd));
+}
+
+
+bool InsetCommandParams::isCompatible(string const & cmd)
+{
+ return command2ParamType(cmd) == type_;
+}
+
+
void InsetCommandParams::setCmdName(string const & name)
{
+ if (!isCompatible(name)) {
+ lyxerr <<
+ "InsetCommandParams: Attempt to convert type of inset from " <<
+ name_ << " to " << name << "." << endl;
+ throw ExceptionMessage(WarningException,
+ _("Invalid type conversion"),
+ from_utf8("Invalid conversion of inset type in InsetCommandParams"));
+ }
name_ = name;
- CommandInfo const * const info = findInfo(name);
+
+ //NOTE The following code is necessary only if we want to allow different
+ //commands associated with a single inset to take different parameters.
+ //No extant inset behaves this way.
+ CommandInfo const * const info = findInfo(name_);
BOOST_ASSERT(info);
ParamVector params(info->n);
- // Overtake parameters with the same name
+ //Get values for new parameters from old ones with the same name
for (size_t i = 0; i < info_->n; ++i) {
int j = findToken(info->paramnames, info_->paramnames[i]);
if (j >= 0)
@@ -260,14 +377,15 @@
{
if (lex.isOK()) {
lex.next();
- name_ = lex.getString();
- info_ = findInfo(name_);
- if (!info_) {
- lex.printError("InsetCommand: Unknown inset name `$$Token'");
+ string name = lex.getString();
+ if (!isCompatible(name)) {
+ lex.printError("InsetCommandParams: Attempt to convert type of inset from " + name_ + " to `$$Token'");
throw ExceptionMessage(WarningException,
- _("Unknown inset name: "),
- from_utf8(name_));
+ _("Invalid type conversion"),
+ from_utf8("Invalid conversion of inset type in InsetCommandParams::read()."));
}
+ name_ = name;
+ info_ = findInfo(name_);
}
string token;
@@ -457,6 +575,7 @@
InsetCommandParams const & o2)
{
return o1.name_ == o2.name_ &&
+ o1.type_ == o2.type_ &&
o1.info_ == o2.info_ &&
o1.params_ == o2.params_ &&
o1.preview_ == o2.preview_;
Index: insetcommandparams.h
===================================================================
--- insetcommandparams.h (revision 17850)
+++ insetcommandparams.h (working copy)
@@ -23,11 +23,15 @@
class LyXLex;
+//FIXME Support for keyval would be nice here and may be necessary
+//for support of some kinds of commands.
class InsetCommandParams {
public:
/// Construct parameters for command \p name. \p name must be known.
explicit InsetCommandParams(std::string const & name);
- ///
+ /// Reads a string representing parameters for this command, as it would
+ /// appear in a LyX file or as entered in the minibuffer, etc. Throws an
+ /// ExceptionMessage if the string is ill-formed.
void read(LyXLex &);
/// Parse the command
/// FIXME remove
@@ -46,9 +50,12 @@
public:
/// FIXME remove
std::string const getContents() const;
- /// Set the name to \p n. This must be a known name. All parameters
- /// are cleared except those that exist also in the new command.
- /// What matters here is the parameter name, not position.
+ /// Set the name to \p n. This must be compatible with the type of inset
+ /// for which this object was originally created, in the sense that it is
+ /// a command valid for that kind of inset. Throws an ExceptionMessage
+ /// otherwise.
+ /// All parameters are cleared except those that exist also in the new
+ /// command. What matters here is the parameter name, not position.
void setCmdName(std::string const & n);
private:
/// FIXME remove
@@ -68,6 +75,9 @@
void preview(bool p) { preview_ = p; }
/// Clear the values of all parameters
void clear();
+ /// Checks if cmd is of the right type for this inset
+ bool isCompatible(std::string const & cmd);
+
private:
///
@@ -79,18 +89,56 @@
/// Tells whether a parameter is optional
bool const * optional;
};
- /// Get information for command \p name.
- /// Returns 0 if the command is not known.
+ /// Get information on what parameters command \p name takes.
+ /// Throws an ExceptionMessage if the command is not known.
static CommandInfo const * findInfo(std::string const & name);
+ //It's not obvious this is the right place for this enum. Note that it
+ //serves a different purpose from the one in insetbase.C and so can't be
+ //replaced by it.
+ /// Types of insets for which we might be parameters
+ enum InsetParamType {
+ ///
+ ICP_BIBITEM,
+ ///
+ ICP_BIBTEX,
+ ///
+ ICP_CITATION,
+ ///
+ ICP_FLOATLIST,
+ ///
+ ICP_HFILL,
+ ///
+ ICP_INCLUDE,
+ ///
+ ICP_INDEX,
+ ///
+ ICP_PRINTINDEX,
+ ///
+ ICP_LABEL,
+ ///
+ ICP_NOMENCL,
+ ///
+ ICP_PRINTNOMENCL,
+ ///
+ ICP_REF,
+ ///
+ ICP_TOC,
+ ///
+ ICP_URL
+ };
+ /// Throws an ExceptionMessage if the command is not known.
+ InsetParamType command2ParamType(std::string const & cmd);
/// Description of all command properties
CommandInfo const * info_;
/// The name of this command as it appears in .lyx and .tex files
std::string name_;
+ /// The type of inset for which we are the parameters
+ InsetParamType type_;
///
typedef std::vector<docstring> ParamVector;
- /// The parameters (both optional and required ones). The order is
- /// the same that is required for LaTeX output. The size of params_
- /// is always info_->n.
+ /// The values of the parameters (both optional and required ones).
+ /// The order is the same that is required for LaTeX output. The size of
+ /// params_ is always info_->n.
ParamVector params_;
///
bool preview_;
