Bo Peng wrote:
Comments welcome.
This is pretty radical. You basically have a relative-path version of
EmbeddedFile in InsetBibtex and convert to EmbeddedFile if needed.
In a way, but there's no EmbeddedFile object stored there at all. Just
the old parameter info. We build EmbeddedFile objects when necessary
mostly so they will tell us about the inzip path and, of course, so we
can use the copyTo() function. If there were a static
EmbeddedFile::getInzipName() function we could call, and if
EmbeddedFileList::registerFile could be called as:
registerFile(pathToFile, inzipName, embeddingStatus, enable);
then we would hardly need to construct them at all. I'll see about that.
This is actually similar to what you have in InsetInclude:
EmbeddedFile const includedFilename(Buffer const & buffer,
InsetCommandParams const & params)
{
// it is not a good idea to create this EmbeddedFile object
// each time, but there seems to be no easy way around.
EmbeddedFile file(to_utf8(params["filename"]),
onlyPath(parentFilename(buffer)));
file.setEmbed(!params["embed"].empty());
file.setInzipName(to_utf8(params["embed"]));
file.enable(buffer.embedded(), &buffer, false);
return file;
}
Much the same idea, and I don't think there's such an issue about
recreating this object. It can't be that expensive. There is still the
problem about filename information being over-written in
updateEmbeddedFile(), though.
By the way, InsetBibtex::setBuffer() never gets called in the existing
code. Inset::setBuffer() gets called instead. My patch fixes that, so
whatever happens, we need that part.
How about making EmbeddedFile a standard-alone class that does not
derive from DocFileName? We simply save whatever path, relative or
absolute a user provide, and get a real FileName when needed. Because
two different absolute paths (external and embedded) are possible,
this does make some sense. This would save us from all the relative
path trouble.
You could do something like that, but I think it's worth disentangling
InsetBibtex from the EmbeddedFiles architecture as much as possible.
Most of my patch just restores the older way of doing things, modified
and cleaned up a bit.
On a slightly different note, I've noticed that when e.g.
LFUN_INSET_MODIFY is called after you, say, change the embedding status
of a .bib file, nothing seems to happen vis a vis the Buffer-level
EmbeddedFileList. The same is true of addDatabase() and delDatabase().
That has to be a bug, though maybe it doesn't matter, since everything
will get revalidated when it needs to be. (If so, though, maybe we don't
need to do it on load.) But if it should be done, then solution might be
to add something like:
embeddedFiles.validate();
at the end of Buffer::updateBibfilesCache(); another and more general
solution might be to have a Buffer::validateEmbeddedFiles() routine we
can call from various places. I'm guessing there's a similar issue with
InsetInclude::setParams(): Presumably the Buffer needs to know about it
if the embedding status has changed.
rh
