Bo Peng wrote:
But I disagree that embedded objects are parameters. In particular, I think
using an EmbeddedFile object to represent a pair of strings is overkill that
ultimately makes the code less comprehensible and less maintainable, because
(a) changes to the EmbeddedFile stuff could in principle affect the inner
workings of the insets and (b) you can't work on the inset without figuring
out the (extremely complex) EmbeddedFile code. (Somewhere in there is the
reason kpsewhich support got broken, and there are still bugs elsewhere that
we have for similar reasons.) As far as I'm concerned, it's no argument that
the EmbeddedFile objects are already lying around. They should do the work
they need to do and leave the rest of the inset alone. And I'd feel exactly
the same way about this if we were talking about InsetBibtexParams. This
issue has nothing to do with ICP.
Unless the current embedding design is revoked (let us discuss this in
another thread), EmbeddedFileList (for a list of embedded bib file),
or EmbeddedFile (for a embedded graphics or included file) have to be
involved in these insets, becuase they dictate how these insets should
behave at runtime. As I have showed you, there is no way to separate
them from the core params,
Yes, we have agreed about that.
and I would like to go further to replace
the core params with EmbeddedFileList in InsetBibtex, as what I have
done for InsetGraphics. This may be complicated, but there is no way
around.
But there is a way around, and it lives in my tree. I thought we'd
agreed about that: we just need to make sure we keep things in sync.
But even if we are going to go more or less the route you suggest, then
there are large-scale questions to be resolved first. If you're going to
subclass InsetCommandParams, then you're going to be making a major
architectural change, and we need to do it carefully. If you're going to
introduce a standalone InsetBibtexParams, then you are going to
duplicate a truckload of code unless we do the same kind of thing for
the rest of them and introduce the "helper methods" Andre keeps talking
about. So please wait on this.
I thought we'd also agreed that you don't need an EmbeddedFileList,
because you don't need any of its methods, etc, etc (and my sense is
that it'd be disastrous to call them). So what you want is just a
vector<EmbeddedFile>. But I thought we'd also agreed that there may be
reason not to use a vector here but instead a map, reasons that have to
do with the bibfilesCache_ (unless you want to build the map as needed).
But a map is no good for the parameters, because order matters. Etc,
etc, all as discussed before.
If you look at the old code, there was a FileNameList that did a lot of
the work your EmbeddedFileList does. But it was NOT used to store the
parameters, and it was NOT iterated over in latex() or anything else.
There were good reasons for that. You replaced the FileNameList with an
EmbeddedFileList, which made some sense, except for the fact that you
don't really want that but just a vector<EmbeddedFile>, as I've just
explained. But then you also used it to store the params and to iterate
over. But if that was a good idea, it should have been a good idea
before to use FileNameList. But it wasn't a good idea then.
rh
