Fwd: [Review requested] New DocBook layout parameter

Tue Dec 27 01:01:25 UTC 2022

On Mon, 26 Dec 2022 at 22:57, Scott Kostyshak <skostysh at lyx.org> wrote:

> On Mon, Dec 26, 2022 at 01:51:11AM +0100, Thibaut Cuvelier wrote:
> > 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):
> >
> > 1663: Test command:
> >
> C:\Users\Thibaut\AppData\Local\JetBrains\Toolbox\apps\CLion\ch-0\223.8214.51\bin\cmake\win\bin\cmake.exe
> > "-DLYX_ROOT=D:/LyX/lyx-unstable/autotests/export"
> > "-DLYX_TESTS_USERDIR=D:/LyX/lyx-unstable/cmake-build-debug/Testing/.lyx"
> > "-Dlyx=D:/LyX/lyx-unstable/cmake-build-debug/bin/LyX.exe"
> > "-DWORKDIR=D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home"
> > "-DLYX_USERDIR_VER=LYX_USERDIR_24x" "-Dformat=lyx23x"
> "-Dextension=23.lyx"
> > "-DLYX_FORMAT_NUM=23" "-Dfile=mixing_inTitle_layouts" "-Dinverted=0"
> > "-DTOP_SRC_DIR=D:/LyX/lyx-unstable"
> > "-DPERL_EXECUTABLE=C:/Strawberry/perl/bin/perl.exe" "-P"
> > "D:/LyX/lyx-unstable/development/autotests/export.cmake"
> > 1663: Working Directory:
> > D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home
> > 1663: Test timeout computed to be: 10000000
> > 1663: -- IgnoreErrorMessage = ""
> > 1663: -- using fonttype =
> > 1663: -- Converting with perl
> > D:/LyX/lyx-unstable/development/autotests/useSystemFonts.pl
> > 1663: -- Using source
> > "D:/LyX/lyx-unstable/autotests/export/mixing_inTitle_layouts.lyx"
> > 1663: -- Using dest
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.lyx"
> > 1663: res1 = 1 for
> > "D:\LyX\lyx-unstable\autotests\export\mixing_inTitle_layouts.lyx" to be
> > copied to
> >
> D:\LyX\lyx-unstable\cmake-build-debug\autotests\out-home\AbC_qo0fts\export\mixing_inTitle_layouts.lyx
> > 1663: -- MD5SUM of
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.lyx"
> > is 543bfbba914aac8e2438f43127d42fb4
> > 1663: -- Executing D:/LyX/lyx-unstable/cmake-build-debug/bin/LyX.exe
> > -userdir "D:/LyX/lyx-unstable/cmake-build-debug/Testing/.lyx" -E lyx23x
> >
> D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.23.lyx
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.lyx"
> > 1663: -- This implicitly checks load of
> >
> D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.lyx
> > 1663: -- Expected result file
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.23.lyx"
> > exists
> > 1663: -- Error line = 'py' is not recognized as an internal or external
> > command,
> > 1663: -- Error line = operable program or batch file.
> > 1663: -- Error line = 'py' is not recognized as an internal or external
> > command,
> > 1663: -- Error line = operable program or batch file.
> > 1663: -- Error line = support\Systemcall.cpp (283): Systemcall: 'git
> > ls-files "mixing_inTitle_layouts.lyx"' did not start!
> > 1663: -- Error line = support\Systemcall.cpp (284): error The process
> > failed to start. Either the invoked program is missing, or you may have
> > insufficient permissions to invoke the program.
> > 1663: -- Msg Summary:
> > 1663: -- OK: Converting
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.lyx"
> > to format lyx23x
> > 1663: -- OK: Structure of the intermediate file
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.23.lyx"
> > 1663: -- Error: Examination of error/warning messages of the conversion
> of
> >
> "D:/LyX/lyx-unstable/cmake-build-debug/autotests/out-home/AbC_qo0fts/export/mixing_inTitle_layouts.lyx"
> > to format lyx23x
> > 1663: -- Exporting export/mixing_inTitle_layouts.lyx to lyx23x
> > 1663: CMake Error at
> > D:/LyX/lyx-unstable/development/autotests/export.cmake:329 (message):
> > 1663:   Export failed
> > 1663:
> > 1663:
> >
> > 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.
>
> Strange indeed. I can't reproduce. Not sure what's going on but it's
> good to know that your patch doesn't change anything.
>

I'm also having strange failures when running the DocBook suite due to Perl
errors (XML parser module could not load, while it seems to be correctly
installed).

> > 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?
>
> I'm fine with whatever Riki prefers since they're handling the beta 2.
>

Riki, let me know :)! I'm attaching a new version of the patch with the
updated layout2layout script (including a change for layout version 98 that
was skipped, if I understand correctly). There is also a second patch that
updates the layouts.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221227/10653dc1/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-DocBook-add-support-for-DocBookGenerateTitle.patch
Type: application/octet-stream
Size: 49364 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221227/10653dc1/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-DocBookGenerateTitle-update-all-layouts.patch
Type: application/octet-stream
Size: 101310 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221227/10653dc1/attachment-0003.obj>