Fwd: [Review requested] New DocBook layout parameter

Mon Dec 26 21:57:34 UTC 2022

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

Scott
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.lyx.org/pipermail/lyx-devel/attachments/20221226/9b520862/attachment.asc>