On Wed, Apr 02, 2008 at 12:52:17PM -0400, Richard Heck wrote:
>> ///
>> static ParamInfo const & findInfo(std::string const &);
>> ///
>> static std::string defaultCommand() { return "href"; };
>> ///
>> static bool isCompatibleCommand(std::string const & s) {
>> return s == "href"; }
>>
>> ParamInfo const & InsetHyperlink::findInfo(string const & /* cmdName
>> */)
>> {
>> static ParamInfo param_info_;
>> if (param_info_.empty()) {
>> param_info_.add("name", ParamInfo::LATEX_OPTIONAL);
>> param_info_.add("target", ParamInfo::LATEX_REQUIRED);
>> param_info_.add("type", ParamInfo::LATEX_REQUIRED);
>> }
>> return param_info_;
>> }
>>
>>
> But (a) none of this code would vanish if you rolled ICP into InsetCommand.
> All of that information would still be needed, and I'd still want at least
> the second and third method to be static.
Why oh why? What's wrong with mathed that it can live without it?
Why would we ever need to have 'isCompatibleCommand'? Just because you
need some kind of call back from the 'generic' ICP::read/write routines?
This is exactly a sign that the stuff is _not_ generic
> And (b), if you're not going to
> roll ICP into the insets (which, see my other message, is not as trivial as
> it might seem), then you're going to need some kind of factory, and that's
> precisely what that code is.
And that's the wrong place. Our core factory is factory.cpp, and for
historical reasons MathFactory.cpp. No need for a new one.
> This was the design choice Georg et al made.
> They could have subclassed ICP and implemented the factory in a different
> way.
No, I need at most inset specific data container which can store
each parameter in its respective "native" format (say lengths as
Length) and not everything in a std::map<std::string, docstring>.
Restricting yourselves to this approach leeds to the kind of trouble
you are currently discussing with Bo: An embedded object seems to
be more than a string, and suddenly major infrastructure changes
_outside_ the "guilty" inset are needed just because it does not fit
well into the abstraction.
> But then (said Georg), you'd need glue code and casts in other places,
> and it seemed simpler (to them) to do it this way. The bigger question has
> to do with having ICP separate from the insets, and I've explained
> elsewhere why it's like that. This is equally true for InsetGraphics as for
> InsetBibtex and has nothing to do with ICP as such.
As I said, ICP once had a very specific use (wrap the data needed to
handle _simple_ latex macros). From there it grew uncontrolled, and now
it seems to dictate architecture. That's _wrong_.
>> You mean I'd end up prefering
>>
>> docstring bibfiles = getParam("bibfiles");
>> docstring embfiles = getParam("embed");
>> // do something with bibfiles and embfiles
>>
>> over
>>
>> // do something with params_.filename (InsetGraphicsParam)
>> [snip]
>> ?
>>
>> That's unlikely.
>>
> You can perfectly well do:
> params_["bibfiles"]
And the proper solution would be:
bibfiles_
> if you like. Both methods are used, for historical reasons, I'd guess. I've
> been trying to clean this up a bit.
I don't see a reason to access something that's conceptionally a simple
C structure as a map with string keys. This is pretty much the same kind
of overengineering we had with the old controllers.
We have
Dimension dim;
dim.wid + dim.asc + dim.des;
and not
Dimension dim;
dim["wid"] + dim["asc"] + dim["des"];
too...
Andre'
