New DocBook support (0005)

Thibaut Cuvelier dourouc05 at gmail.com
Mon Jul 6 03:18:16 UTC 2020


On Sun, 5 Jul 2020 at 11:59, Pavel Sanda <sanda at lyx.org> wrote:

> On Sat, Jul 04, 2020 at 12:42:31AM +0200, Thibaut Cuvelier wrote:
> > From f7004ab6518c230f6cedd89410d9059e9596a113 Mon Sep 17 00:00:00 2001
> > From: Thibaut Cuvelier <cuvelier.thibaut at gmail.com>
> > Date: Mon, 8 Jun 2020 23:27:49 +0200
> > Subject: [PATCH 5/9] New DocBook support
>


> > -Buffer::ExportStatus Buffer::writeDocBookSource(odocstream & os, string
> const & fname,
> > +Buffer::ExportStatus Buffer::writeDocBookSource(odocstream & os, string
> const & /*fname*/,
>
> time to get rid of string const & /*fname*/ altogether?
>

Done.

the comments should be rather next to the variables below
>

Done.


> >  //  The order of the LayoutTags enum is no more important.
> [asierra300396]
> > @@ -104,6 +104,21 @@ enum LayoutTags {
> >       LT_HTMLPREAMBLE,
> >       LT_HTMLSTYLE,
> >       LT_HTMLFORCECSS,
> > +     LT_DOCBOOKTAG,
> > +     LT_DOCBOOKATTR,
> > +    LT_DOCBOOKININFO,
> > +     LT_DOCBOOKWRAPPERTAG,
> > +     LT_DOCBOOKWRAPPERATTR,
> > +     LT_DOCBOOKSECTIONTAG,
> > +    LT_DOCBOOKITEMWRAPPERTAG,
> > +    LT_DOCBOOKITEMWRAPPERATTR,
> > +    LT_DOCBOOKITEMTAG,
> > +    LT_DOCBOOKITEMATTR,
> > +    LT_DOCBOOKITEMLABELTAG,
> > +    LT_DOCBOOKITEMLABELATTR,
> > +     LT_DOCBOOKITEMINNERTAG,
> > +     LT_DOCBOOKITEMINNERATTR,
> > +     LT_DOCBOOKFORCEABSTRACTTAG,
>
> ditto
>

What do you mean? Indentation problems? If so, done.


> > @@ -204,6 +219,21 @@ bool Layout::readIgnoreForcelocal(Lexer & lex,
> TextClass const & tclass)
> >               { "commanddepth",   LT_COMMANDDEPTH },
> >               { "copystyle",      LT_COPYSTYLE },
> >               { "dependson",      LT_DEPENDSON },
> > +             { "docbookattr",             LT_DOCBOOKATTR },
> > +             { "docbookforceabstracttag", LT_DOCBOOKFORCEABSTRACTTAG },
> > +        { "docbookininfo",           LT_DOCBOOKININFO },
> > +        { "docbookitemattr",         LT_DOCBOOKITEMATTR },
> > +             { "docbookiteminnerattr",    LT_DOCBOOKITEMINNERATTR },
> > +             { "docbookiteminnertag",     LT_DOCBOOKITEMINNERTAG },
> > +             { "docbookitemlabelattr",    LT_DOCBOOKITEMLABELATTR },
> > +             { "docbookitemlabeltag",     LT_DOCBOOKITEMLABELTAG },
> > +        { "docbookitemtag",          LT_DOCBOOKITEMTAG },
> > +        { "docbookitemwrapperattr",  LT_DOCBOOKITEMWRAPPERATTR },
> > +        { "docbookitemwrappertag",   LT_DOCBOOKITEMWRAPPERTAG },
> > +             { "docbooksectiontag",       LT_DOCBOOKSECTIONTAG },
> > +             { "docbooktag",              LT_DOCBOOKTAG },
> > +             { "docbookwrapperattr",      LT_DOCBOOKWRAPPERATTR },
> > +             { "docbookwrappertag",       LT_DOCBOOKWRAPPERTAG },
> >               { "end",            LT_END },
> >               { "endlabelstring", LT_ENDLABELSTRING },
> >               { "endlabeltype",   LT_ENDLABELTYPE },
>
> ditto
>

Just indentation? If so, done.


> > @@ -689,6 +719,66 @@ bool Layout::readIgnoreForcelocal(Lexer & lex,
> TextClass const & tclass)
> > +             case LT_DOCBOOKWRAPPERATTR:
> > +                     lex >> docbookwrapperattr_;
> > +                     break;
> > +
> > +             case LT_DOCBOOKSECTIONTAG:
> > +                     lex >> docbooksectiontag_;
> > +                     break;
> > +
> > +        case LT_DOCBOOKITEMWRAPPERTAG:
> > +            lex >> docbookitemwrappertag_;
> > +            break;
> > +
> > +        case LT_DOCBOOKITEMWRAPPERATTR:
> > +            lex >> docbookitemwrapperattr_;
> > +            break;
> > +
> > +             case LT_DOCBOOKITEMTAG:
> > +                     lex >> docbookitemtag_;
> > +                     break;
> > +
>
> ditto


Just indentation? If so, done.


> >  string makeMarginValue(char const * side, double d)
> > diff --git a/src/Layout.h b/src/Layout.h
> > index ffc976d8ff..bfcb510861 100644
> > --- a/src/Layout.h
> > +++ b/src/Layout.h
> > @@ -193,6 +193,36 @@ public:
> >       ///
> >       bool htmltitle() const { return htmltitle_; }
> >       ///
> > +     std::string const & docbookattr() const;
> > +     ///
> > +     std::string const & docbookininfo() const;
> > +    ///
> > +    std::string const & docbookwrappertag() const;
> > +    ///
> > +    std::string const & docbookwrapperattr() const;
> > +    ///
> > +    std::string const & docbooksectiontag() const;
> > +    ///
> > +    std::string const & docbookitemwrappertag() const;
> > +    ///
> > +    std::string const & docbookitemwrapperattr() const;
> > +    ///
> > +    std::string const & docbookitemlabeltag() const;
> > +    ///
> > +    std::string const & docbookitemlabelattr() const;
> > +     ///
> > +     std::string const & docbookiteminnertag() const;
>
> ditto
>

Just indentation? If so, done.


> > @@ -457,6 +487,39 @@ private:
> >       bool htmllabelfirst_;
> >       /// CSS information needed by this layout.
> >       docstring htmlstyle_;
> > +     mutable std::string docbookitemtag_;
> > +     /// Roles to add to docbookitemtag_, if any (default: none).
> > +     mutable std::string docbookitemattr_;
> > +    /// Tag corresponding to the wrapper around an item (mainly for
> lists).
> > +    mutable std::string docbookitemwrappertag_;
> > +    /// Roles to add to docbookitemwrappertag_, if any (default: none).
> > +    mutable std::string docbookitemwrapperattr_;
> > +    /// Tag corresponding to this label (only for description lists;
> > +    /// labels in the common sense do not exist with DocBook).
> > +     mutable std::string docbookitemlabeltag_;
> > +     /// Roles to add to docbooklabeltag_, if any (default: none).
> > +     mutable std::string docbookitemlabelattr_;
> > +     /// Tag to add within the item, around its direct content (mainly
> for lists).
> > +     mutable std::string docbookiteminnertag_;
> > +     /// Roles to add to docbookiteminnertag_, if any (default: none).
> > +     mutable std::string docbookiteminnerattr_;
> > +    /// Tag corresponding to this wrapper around the main tag.
> > +    mutable std::string docbookwrappertag_;
> > +    /// Roles to add to docbookwrappertag_, if any (default: none).
> > +    mutable std::string docbookwrapperattr_;
> > +    /// Outer tag for this section, only if this layout represent a
> sectionning item, including chapters (default: section).
> > +    mutable std::string docbooksectiontag_;
> > +    /// Whether this tag must/can/can't go into an <info> tag (default:
> never, as it only makes sense for metadata).
> > +     mutable std::string docbookininfo_;
> > +    /// whether this element (root or not) does not accept text without
> a section(i.e. the first text that is met
> > +     /// in LyX must be considered as the abstract if this is true);
> this text must be output with the specific tag
> > +     /// held by this attribute
> > +    mutable std::string docbookforceabstracttag_;
>
> ditto
>

Just indentation (comments being near the attributes, not the methods)? If
so, done.


> > 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.


> >+void Paragraph::simpleDocBookOnePar(Buffer const & buf,
> ...
> > +        // FIXME XHTML
> > +        // Other such tags? What about the other text ranges?
> > +
> > +        vector<xml::EndFontTag>::const_iterator cit =
> tagsToClose.begin();
> > +        vector<xml::EndFontTag>::const_iterator cen = tagsToClose.end();
> > +        for (; cit != cen; ++cit)
> > +            xs << *cit;
> ...
>
> > +    // 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.


> > +    void simpleDocBookOnePar(Buffer const & buf,
> > +                                                      XMLStream &,
> > +                                          OutputParams const &
> runparams,
> > +                                          Font const & outerfont,
> > +                             bool start_paragraph = true,
> > +                             bool close_paragraph = true,
> > +                                          pos_type initial = 0) const;
>
> whitespace
>

Done.


> > diff --git a/src/insets/InsetBibtex.cpp b/src/insets/InsetBibtex.cpp
> > index 3bfc593013..368d4f7ef6 100644
> > --- a/src/insets/InsetBibtex.cpp
> > +++ b/src/insets/InsetBibtex.cpp
> > @@ -51,6 +50,11 @@
> >  #include "support/textutils.h"
> >
> >  #include <limits>
> > +#include <map>
> > +#include <regex>
>
> Careful here. I was never part of that discussion but there were some
> issues with regex implementation for different compilers so we provide
> #include "support/regex.h"
> and I think it should be used that across our codebase consistently.
> (someone else might chime in?)
>

Done.


> > +void InsetBibtex::docbook(XMLStream & xs, OutputParams const &) const
> > +     // Header for bibliography (title required).
> > +     xs << xml::StartTag("bibliography") << xml::CR()
> > +        << xml::StartTag("title")
> > +        << reflabel
> > +        << xml::EndTag("title") << xml::CR();
>
> chains everywhere in this function..
>

Done.


> > +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)?


> > +void InsetFloat::docbook(XMLStream & xs, OutputParams const &
> runparams) const
> > +{
> > +     // Determine whether the float has a title or not. For this,
> iterate through the paragraphs and look
> > +    // for an InsetCaption. Do the same for labels and subfigures.
> > +    // The caption and the label for each subfigure is handled by
> recursive calls.
> > +    const InsetCaption* caption = nullptr;
> > +    const InsetLabel* label = nullptr;
> > +    std::vector<const InsetBox *> subfigures;
> > +
> > +    auto it = paragraphs().begin();
> > +    auto end = paragraphs().end();
> > +     for (; it != end; ++it) {
>
> whitespace
>

Done.


> > +    if (!ftype.docbookAttr().empty()) {
> > +        if (!attr.empty())
> > +            attr += " ";
> > +        attr += from_utf8(ftype.docbookAttr());
> > +    }
> > +
> > +     xs << xml::StartTag(ftype.docbookTag(caption != nullptr), attr) <<
> xml::CR();
> > +    if (caption != nullptr) {
>
> ditto
>

Done.


> > diff --git a/src/insets/InsetGraphics.cpp b/src/insets/InsetGraphics.cpp
> > index 0daae4c921..50a5b58fa4 100644
> > --- a/src/insets/InsetGraphics.cpp
> > +++ b/src/insets/InsetGraphics.cpp
> > @@ -96,6 +96,7 @@ TODO
> >  #include <algorithm>
> >  #include <sstream>
> >  #include <tuple>
>
> unrelated, but i wonder whether tupple is ever used here
>

It doesn't look like it is, so removed.


> > +void InsetHyperlink::docbook(XMLStream & xs, OutputParams const &) const
> >  {
> > -     os << "<ulink url=\""
> > -        << subst(getParam("target"), from_ascii("&"),
> from_ascii("&"))
> > -        << "\">"
> > +     xs << xml::StartTag("link", "xlink:href=\"" +
> subst(getParam("target"), from_ascii("&"), from_ascii("&")) + "\"")
> >          << xml::escapeString(getParam("name"))
> > -        << "</ulink>";
> > -     return 0;
> > +        << xml::EndTag("link");
>
> chains
>

Done.


> > +             // Handle the index terms (including the specific index
> for this entry).
> > +             xs << xml::StartTag("indexterm", attrs);
> > +             if (terms.size() > 0) { // hasEndRange has no content.
> > +                     xs << xml::StartTag("primary")
> > +                        << terms[0]
> > +                        << xml::EndTag("primary");
> > +             }
> > +             if (terms.size() > 1) {
> > +                     xs << xml::StartTag("secondary")
> > +                        << terms[1]
> > +                        << xml::EndTag("secondary");
> > +             }
> > +             if (terms.size() > 2) {
> > +                     xs << xml::StartTag("tertiary")
> > +                        << terms[2]
> > +                        << xml::EndTag("tertiary");
> > +             }
> > +
> > +             // Handle see and see also.
> > +             if (!see.empty()) {
> > +                     xs << xml::StartTag("see")
> > +                        << see
> > +                        << xml::EndTag("see");
> > +             }
> > +
> > +             if (!seeAlsoes.empty()) {
> > +                     for (auto & entry : seeAlsoes) {
> > +                             xs << xml::StartTag("seealso")
> > +                                << entry
> > +                                << xml::EndTag("seealso");
>
> chains
>

Done.


> > +void InsetListings::docbook(XMLStream & xs, OutputParams const & rp)
> const
> > +     if (!caption.empty())
> > +             xs << xml::StartTag("bridgehead")
> > +                << XMLStream::ESCAPE_NONE
> > +                << caption
> > +                << xml::EndTag("bridgehead");
>
> chains
>

Done.


> > +void InsetNewline::docbook(XMLStream & xs, OutputParams const &
> runparams) const
> >  {
> > -     os << '\n';
> > -     return 0;
> > +     if (runparams.docbook_in_par) {
> > +             xs.closeFontTags();
> > +             xs << xml::EndTag("para");
> > +             xs << xml::StartTag("para");
>
> chains
>

Done.


> > -// FIXME This should be changed to use the TOC. Perhaps
> > -// that could be done when XHTML output is added.
> > -int InsetPrintNomencl::docbook(odocstream & os, OutputParams const &)
> const
> > +void InsetPrintNomencl::docbook(XMLStream & xs, OutputParams const &
> runparams) const
> ..
> > +     xs << xml::StartTag("glossary")
> > +        << xml::CR()
> > +        << xml::StartTag("title")
> > +        << toclabel
> > +        << xml::EndTag("title")
> > +        << xml::CR();
> > +
> > +     EntryMap::const_iterator eit = entries.begin();
> > +     EntryMap::const_iterator const een = entries.end();
> > +     for (; eit != een; ++eit) {
> > +             NomenclEntry const & ne = eit->second;
> > +             string const parid = ne.par->magicLabel();
> > +
> > +             xs << xml::StartTag("glossentry", "xml:id=\"" + parid +
> "\"")
> > +                << xml::CR()
> > +                << xml::StartTag("glossterm")
> > +                << ne.symbol
> > +                << xml::EndTag("glossterm")
> > +                << xml::CR()
> > +                << xml::StartTag("glossdef")
> > +                << xml::CR()
> > +                << xml::StartTag("para")
> > +                << ne.desc
> > +                << xml::EndTag("para")
> > +                << xml::CR()
> > +                << xml::EndTag("glossdef")
> > +                << xml::CR()
> > +                << xml::EndTag("glossentry")
> > +                << xml::CR();
>
> chains
>

Done.


> > -int InsetRef::docbook(odocstream & os, OutputParams const & runparams)
> const
> > +void InsetRef::docbook(XMLStream & xs, OutputParams const &) const
> ..
> > +     if (!name.empty()) {
> > +             docstring attr = from_utf8("linkend=\"") + linkend +
> from_utf8("\"");
> > +
> > +             xs << xml::StartTag("link", to_utf8(attr))
> > +                << name
> > +                << xml::EndTag("link");
> > +             return;
>
> chains
>

Done.

> --- a/src/output_docbook.h
> > +++ b/src/output_docbook.h
> > +#include "support/docstream.h"
> ...
> > +#include <deque>
> > +#include <memory>
>
> Are these includes necessary in the header?
>

Done. Actually, they were not even required in the .cpp, probably
left-overs.


> > --- a/src/xml.cpp
> > +++ b/src/xml.cpp
> > @@ -32,6 +32,7 @@
> > +#include <iostream>
>
> used?
>

Removed.

Here is an updated patch.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20200706/33a526ba/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-New-DocBook-support.patch
Type: application/octet-stream
Size: 222592 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20200706/33a526ba/attachment-0001.obj>


More information about the lyx-devel mailing list