New DocBook support (0005)

Pavel Sanda sanda at lyx.org
Mon Jul 6 10:45:06 UTC 2020


On Mon, Jul 06, 2020 at 05:18:16AM +0200, Thibaut Cuvelier wrote:
> > > diff --git a/src/OutputParams.h b/src/OutputParams.h
> > > index 1ad36722d0..0244a0ea41 100644
> > > --- a/src/OutputParams.h
> > > +++ b/src/OutputParams.h
> > > @@ -16,6 +16,7 @@
> > >  #include "Changes.h"
> > >
> > >  #include <memory>
> > > +#include <unordered_set>
> > >
> > > +     /// Anchors that should not be output (LyX-side identifier, not
> > DocBook-side).
> > > +     std::unordered_set<docstring> docbook_anchors_to_ignore;
> >
> > Adding new deps into .h files is not a best things for compilation speeds.
> > Won't std::vector already included from Changes.h be enough (do we care
> > about uniqness)?
> >
> 
> Uniqueness is not important; however, checking for inclusion within the set
> is important: actually, it's the only operation performed afterwards. With
> a vector, this would have a linear complexity, and it could not be
> amortised. With a set, it should be on average constant time (at least,
> that's why cppreference says, I don't know what the standard mandates as
> complexity) ??? unless the user defeats the hashing algorithm, but I'm not
> sure we need to take care of that edge case.

I see, if n is the size of input document the numbers of anchors are O(n)
and number of queries on the set is O(n) as well?

> > > +    // FIXME XHTML
> > > +    // I'm worried about what happens if a branch, say, is itself
> > > +    // wrapped in some font stuff. I think that will not work.
> > > +    xs.closeFontTags();
> >
> > Just to clarify FIXME XHTML is here because we need to fix this
> > in other routines implementing xHTML or because this routine is shared?
> >
> 
> It's because this code was copied and only lightly adapted from the XHTML
> code path.

So "FIXME XHTML" means "FIXME, this code is just imported from XHTML"
or "FIXME, this code needs fix both here and in XHTML" ?

> > > +void InsetCaption::docbook(XMLStream &, OutputParams const &) const
> > >  {
> > > -     int ret;
> > > -     os << "<title>";
> > > -     ret = InsetText::docbook(os, runparams);
> > > -     os << "</title>\n";
> > > -     return ret;
> > > +     // This function should never be called (rather
> > InsetFloat::docbook, the titles should be skipped in floats).
> >
> > adding LYXERRR warning?
> >
> 
> Guillaume told me this was more intended to display comments to the user
> (in a dialog, within the GUI). Shouldn't this rather be a warning output in
> the XML file? Or just a message in the console (if so, how)?

Naively I would expect that InsetCaption::docbook processes the text in the
caption of floats (e.g. insert figure float and the inset which you see just
behind "Figure 1:" label.), but if it's handled in InsetFloat::docbook then ok.

In such case I would just issue console warning via 
LYXERR0("InsetCaption::docbook should be never called!");

> Here is an updated patch.

For some reason, unlike the initial emails where the attachments were Type: application/x-patch
this one is Type: application/octet-stream, which makes my email reader treat is as a binary not
to be shown (I can handle it is but it's nuisance). Perhaps you are changing email clients
or something of the sort(?).


Otherwise it seems we are close to commit, I'll probably do one more lengthy
pass only after commit.

Thanks,
Pavel


More information about the lyx-devel mailing list