Am Sonntag, 22. April 2007 15:59 schrieb Richard Heck:
> Georg Baum wrote:
> > - Change the InsetComnmandParams constructor to take an InsetBase::Code
> > instead of a name. name_ would simply be set to the first possible name
of
> > that inset.
> >
> There's no need to worry about name_, really. It gets re-written almost
> immediately, when the parameters are read.
But this is not guaranteed. One could create a params instance without
calling read() later, so name_ should be set in the constructor. This
might look pedantic, but it is cheap, and being correct also in the corner
cases is worth it in my experience.
> But is it worth changing the
> constructor and making extensive changes, e.g., in factory.C?
I think so, because then we would only need a map from code to name.
Otherwise we would also need one from name to code.
> If it
> seems best, then of course those changes can be made. Alternatively,
> name can continue to be passed, and the InsetBase::Code determined by
> that name can then be stored for later use. I'm happy to do it either
way.
>
> I'll also need to add the other possible names to the enum in InsetBase,
> but we'll have to do that anyway, if the InsetBase::Code is to be used
> for error checking, as below.
Why would you need to add any name? My proposal was not to replace name_ by
the new member. If you store name_ (as it is now), but have in addition a
new member code_, then findInfo would become a big switch, testing for
code_ and returning the right info:
switch (code_) {
case BIBITEM_CODE:
static const char * const paramnames[] = {"label", "key", ""};
static const bool isoptional[] = {true, false};
static const CommandInfo info = {2, paramnames, isoptional};
return &info;
case ...
}
The list of command names that is currently in findInfo could be put into a
new method isCompatible: It would contain a similar switch, but this time
testing the names:
switch (code_) {
case BIBITEM_CODE:
if (name_ == "bibitem")
return true;
case ...
}
> I worry about this, and my earlier proposal, because the list of
> acceptable commands is hard-coded. This is fine for existing insets, but
> could be a problem, again, for BibLaTeX support, since BibLaTeX does not
> pre-define the citation commands. So here's a different idea. As you
> said before, it seems as if the inset ought really to handle this kind
> of thing. The inset ought to know what commands are OK. Part of the
> problem is that, at present, an inset's parameters can be changed
> without its knowledge: You just call inset->params(), and then do as you
> please, and the params don't know to which inset they belong. But, on
> your proposal, the params do know to what kind of inset they belong. So
> what if each inset had a static method isAcceptableCommand(string const
> &) that could be called in findInfo() and setCmdName()?
Yes, that would work, if you would use a similar switch as above, but it
would add more line noise. Another alternative would be a mixture: Use the
static method only for those insets that do not have hardcoded names.
> Alternatively, I
> suppose, the params could have an owner_, which would get set in the
> inset constructor. But that involves more changes.
Such a back pointer would be difficult to keep up to date.
> I'll proceed with some version of this, once we're fully agreed how to
> do it.
I guess I made my concerns clear enough now, so please decide yourself what
you want to implement. I don't want to hear again that I tell other people
what they should do or not do :-)
Georg
