openSUSE / geekodoc

RELAX NG Schema for SUSE Documentation
https://opensuse.github.io/geekodoc
GNU General Public License v3.0
4 stars 5 forks source link

formalpara can be used as first element of an admon #60

Closed ghost closed 1 year ago

ghost commented 5 years ago

In Geekodoc, you can currently use formalpara as the first element of an admon:

<important>
  <formalpara><title>bla</title><para>blub</para></formalpara>
</important>

or

<important>
  <title>haha jk</title>
  <formalpara><title>bla</title><para>blub</para></formalpara>
</important>

Neither construct makes any sense to me. Could we restrict this?

cf. the online-docs failure of the CAP guide, develop@4e27a3e6.

tomschr commented 5 years ago

Good catch! Currently, admonitions have these content model (resolved):

db.admonition.contentmodel = db._info.title.only, 
   (db.list.blocks
     | db.formal.blocks   # db.example | db.figure | db.table | db.equation
     | db.informal.blocks   # db.informalexample | db.informalfigure | db.informaltable  | db.informalequation
     | db.publishing.blocks  # db.sidebar | db.blockquote | db.address | db.epigraph
     | db.graphic.blocks  # db.mediaobject | db.screenshot
     | db.technical.blocks  # db.procedure | db.task | db.productionset | db.constraintdef | db.msgset
     | db.verbatim.blocks  # db.screen | db.literallayout  | db.programlistingco | db.screenco  | db.programlisting | db.synopsis
     | db.bridgehead
     | db.remark
     | db.revhistory
     | db.indexterm
     | db.synopsis.blocks
     | db.admonition.blocks
     | db.anchor | db.para | db.formalpara | db.simpara
     | db.extension.blocks)+
   | db.annotation
   | db.xi.include

That would be a good opportunity not only remove formalpara, but to consolidate the content model of admonitions too. I propose the following model:

db.admonition.contentmodel = db._info.title.only, 
    (db.list.blocks
     | db.verbatim.blocks
     | db.remark
     | db.para
     | db.xi.include
     )+

I'm not entirely sure about graphics (db.informal.blocks and db.graphic.blocks) inside admonitions.

@sknorr What do you think? Does it look ok?

ghost commented 5 years ago

@tomschr Not sure if I answered in another way or did not answer at all, but \<formalpara/> should be allowed in general. However, as the first element after the title, it makes no sense. Essentially, I'd leave most of the model intact, except I'd force an initial para.

What I completely removed v/ upstream are essentially just four things:

What I was unsure about: What are extension.blocks and anchor?

The following model would make more sense to me:

db.admonition.contentmodel = db._info.title.only,
    (db.remark?,
    (db.para
     | db.xi.include
     ),
    (db.list.blocks
     | db.informal.blocks   # db.informalexample | db.informalfigure | db.informaltable  | db.informalequation
     | db.publishing.blocks  # db.sidebar | db.blockquote | db.address | db.epigraph
     | db.graphic.blocks  # db.mediaobject | db.screenshot
     | db.technical.blocks  # db.procedure | db.task | db.productionset | db.constraintdef | db.msgset
     | db.verbatim.blocks  # db.screen | db.literallayout  | db.programlistingco | db.screenco  | db.programlisting | db.synopsis
     | db.remark
     | db.anchor | db.para | db.formalpara | db.simpara
     | db.extension.blocks)*),
   | db.xi.include
tomschr commented 5 years ago

Thanks @sknorr for the suggestion! :+1: After looking at the pull request #67 again, your suggestions to force a para as a first child makes sense to me. Like it! :+1:

I would have only a slight change/wish: Could we change the db.remark? to db.remark*? IMHO, it should be ok to use more than one remark, if necessary. Or do you have a special use case in your mind?

What I was unsure about: What are extension.blocks and anchor?


I observed a syntax error with your content model, so I'd propose to use this (clarified and collected the removed patterns in one location):

db.admonition.contentmodel =
      db._info.title.only,
      (db.remark*,
       (db.para | db.xi.include),
       # Removed patterns:
       # db.informalexample | db.informalfigure | db.informaltable  | db.informalequation
       # db.sidebar | db.blockquote | db.address | db.epigraph
       # db.mediaobject | db.screenshot
       # db.procedure | db.task | db.productionset | db.constraintdef | db.msgset
       # db.screen | db.literallayout  | db.programlistingco | db.screenco  | db.programlisting | db.synopsis
       (db.list.blocks
       | db.informal.blocks
       | db.publishing.blocks
       | db.graphic.blocks
       | db.technical.blocks
       | db.verbatim.blocks
       | db.remark
       | db.para | db.formalpara | db.simpara
       | db.extension.blocks
       | db.xi.include)*
      )

Would you like to review commit 0b7a1ec?

ghost commented 5 years ago

Uh, sorry for probably missing this initially.

Looks good, except: can you please enable the elements db.informalexample | db.informalfigure | db.informaltable | db.informalequation again?

tomschr commented 5 years ago

Uh, sorry for probably missing this initially.

Na, I also forgot about it. :wink:

Looks good, except: can you please enable the elements db.informalexample | db.informalfigure | db.informaltable | db.informalequation again?

Although I was originally against the idea of adding these elements into admons (to blow up the content), I'll add them anyway. See it in commit 1928040. But keep in mind, db.informalequation is currently set as notAllowed. I think, we can live without an equation for the time being. :wink:

ghost commented 5 years ago

Also, I am a bit confused, your comment in the commit says: "Removed elements [...] db.screen" but db.verbatim.blocks is included -- or are these not the same? (I think we need screen within admons, so if my observation is correct, please just fix the comment.)

tomschr commented 5 years ago

Yes, db.screen is included in db.verbatim.blocks, so screen is not removed. Don't worry. :wink:

The original DocBook contained in db.verbatim.blocks: literalllayout, programlisting/programlistingco, screen/screenco and synopsis. It is a pattern which contains all so-called "verbatim" elements which preserves whitespace.

Yeah, maybe I should fix/remove/adapt the comment.

But I realized, I didn't add that as a test case (i think, that is really important). Thanks for the hint! :+1:

tomschr commented 1 year ago

Jana and I decided it's overkill and we leave it as is.