[Review requested] New DocBook layout parameter

[Kornel Benko]

Am Mon, 26 Dec 2022 01:51:11 +0100
schrieb Thibaut Cuvelier <dourouc05 at gmail.com>:

> On Sat, 24 Dec 2022 at 03:45, Scott Kostyshak <skostysh at lyx.org> wrote:
> 
> > On Thu, Dec 22, 2022 at 04:45:52AM +0100, Thibaut Cuvelier wrote:
> > > Dear list,
> > >
> > > To solve https://www.lyx.org/trac/ticket/12612, I needed a new layout
> > > parameter for DocBook to generate some content based on paragraph labels.
> > > It draws heavily from LyXHTML code: makeEnvironment in output_xhtml.cpp,
> > > case on environments, label generation (currently, lines 499 to 511).
> > >
> > > As I'm still not very confident with layout changes, and especially as
> > LyX
> > > master is maturing, I'm requesting some feedback/review before pushing. I
> > > believe everything is there (code changes, layout version update, changes
> > > in the layout to use the new feature, documentation), with the exception
> > of
> > > updating the version number of all layouts (to avoid cluttering the
> > patch).
> > >
> > > By the way, I noticed that the documentation on the new DocBook
> > parameters
> > > for the layouts is truly lacking. I'll try to do something about it.
> > >
> > > Thanks in advance!
> > >
> > > Thibaut Cuvelier
> >
> > Hi Thibaut,
> >
> > Thank you for working on that, and proposing to the list. I don't
> > understand the patch well, but from what I understand the C++ changes
> > are small and your main concern is just making sure you checked all the
> > boxes for what to do with a layout change. Unless anyone else has
> > comments, I would say that after beta2 is tagged go ahead and push it if
> > all of the docbook tests pass. Maybe also run <<ctest -R "layout">>.
> > There are some layout tests that run quickly, although I forget what
> > they do.
> >
> 
> Thanks!
> 
> I'm having some failures that I don't really understand (I'm skipping the
> UNRELIABLE tests):
> 
> The following tests FAILED:
> 1659 - export/export/mixing_inTitle_layouts_lyx16 (Failed)
> 1660 - export/export/mixing_inTitle_layouts_lyx20 (Failed)
> 1661 - export/export/mixing_inTitle_layouts_lyx21 (Failed)
> 1662 - export/export/mixing_inTitle_layouts_lyx22 (Failed)
> 1663 - export/export/mixing_inTitle_layouts_lyx23 (Failed)
> 1665 - check_load/export/mixing_inTitle_layouts (Failed)
> 
> Here is the output for one test (lyx23):
...

I cannot reproduce here. These tests pass on my machine.

> What's strange is that I cannot reproduce when I start LyX, because that
> export document exports to LyX 2.3 format without problems through the GUI
> (File > Export > 2.3).
> LyX recognises Python in the PATH (otherwise, 60 more tests fail).
> These failures should not be caused by my patch, as they fail without it.
> 
> I'm attaching a new version of the patches that does not conflict with the
> current state of master. @JMarc: it includes your comment on the default
> value :)!
> 
> Why would we wait until after beta 2? Isn't the goal of a beta to gather as
> many new changes to catch as many regressions as possible?

	Kornel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: Digitale Signatur von OpenPGP
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221226/2eccdaf2/attachment.asc>