lyx2lyx roundtrip fails for Embedded Objects manual

[Scott Kostyshak]

On master branch, if I open the embedded objects manual, I can compile
with pdflatex. If I then export the manual to 2.0.x format and then
reopen the exported file in the master branch, I get the following
error:

! Package xcolor Error: Undefined color `Color'.

Is this finding interesting/useful?
If so, I will test all manuals. Any roundtrip requests? Right now I'm
thinking of master branch's format <-> 413.

Scott

[Richard Heck]

Post by Scott Kostyshak
On master branch, if I open the embedded objects manual, I can compile
with pdflatex. If I then export the manual to 2.0.x format and then
reopen the exported file in the master branch, I get the following
! Package xcolor Error: Undefined color `Color'.
Is this finding interesting/useful?
If so, I will test all manuals. Any roundtrip requests? Right now I'm
thinking of master branch's format <-> 413.

On the wiki, there's a page about release procedures. Can you add a note
to the effect that
we should run all these tests before we release a new major version?

Richard

[Scott Kostyshak]

Post by Richard Heck
On the wiki, there's a page about release procedures.

Do you mean the following?
http://wiki.lyx.org/Devel/ReleaseProcedure
Does anything need to be updated on that page for 2.1.x? (it is
talking about 2.0.0 and 2.0.x)

Post by Richard Heck
Can you add a note to
the effect that
we should run all these tests before we release a new major version?

Sure, but which tests are you talking about?

A simple and fast set of tests that could be run is to use lyx2lyx to
do the roundtrip but not to export. Perhaps this is what you're
referring to? Now that I reread my original email, this is what it
sounds like I did. But really what I did was this and then I ran the
export tests. Using only lyx2lyx would rely on lyx2lyx error reporting
so if that is likely to catch errors then it might be worth it.

Scott

[Richard Heck]

Post by Scott Kostyshak

Post by Richard Heck
On the wiki, there's a page about release procedures.

Do you mean the following?
http://wiki.lyx.org/Devel/ReleaseProcedure
Does anything need to be updated on that page for 2.1.x? (it is
talking about 2.0.0 and 2.0.x)

Post by Richard Heck
Can you add a note to
the effect that
we should run all these tests before we release a new major version?

Sure, but which tests are you talking about?
A simple and fast set of tests that could be run is to use lyx2lyx to
do the roundtrip but not to export.

Yes. We should check that the exported files open properly in the
previous version, and that
the roundtrip files open properly in the new one.

rh

[Scott Kostyshak]

Post by Scott Kostyshak
A simple and fast set of tests that could be run is to use lyx2lyx to
do the roundtrip but not to export.

Yes. We should check that the exported files open properly in the previous
version, and that
the roundtrip files open properly in the new one.

OK so use lyx2lyx and then check that LyX opens the files without
writing any output to standard error. Makes sense and is easy and fast
to automate.

Scott

[Georg Baum]

Post by Scott Kostyshak
A simple and fast set of tests that could be run is to use lyx2lyx to
do the roundtrip but not to export. Perhaps this is what you're
referring to? Now that I reread my original email, this is what it
sounds like I did. But really what I did was this and then I ran the
export tests. Using only lyx2lyx would rely on lyx2lyx error reporting
so if that is likely to catch errors then it might be worth it.

My tests that eventually led to 2.0.8.1 where quite simple:

rm -f x.diff
for i in ../../lyx-2.0/lib/doc/*.lyx
do
bn=`basename $i`
python ../lib/lyx2lyx/lyx2lyx < $i > $bn-474.lyx
python ../lib/lyx2lyx/lyx2lyx -t 413 < $bn-474.lyx > $bn-413.lyx
diff -u $i $bn-413.lyx >> x.diff
done

I did not even bother to write a script but simply typed the above commands
into a shell, and then looked at x.diff

This does not need any error reporting. Of course the test might be too
strict (e.g. in some cases you'll get empty lines added or removed, which do
not matter), but for the 2.1->2.0->2.1 roundtrip it worked well.

We definitely need automatic lyx2lyx tests, and it would be really nice if
somebody could think a bit how they work best and then implement that.


Georg

[Richard Heck]

Post by Scott Kostyshak
On master branch, if I open the embedded objects manual, I can compile
with pdflatex. If I then export the manual to 2.0.x format and then
reopen the exported file in the master branch, I get the following
! Package xcolor Error: Undefined color `Color'.
Is this finding interesting/useful?

Oh, and on this score: Can you tell why we get the error? One way to
localize it would be
to bisect on the format to which you are converting.

Richard

[Enrico Forestieri]

Post by Richard Heck

Post by Scott Kostyshak
On master branch, if I open the embedded objects manual, I can compile
with pdflatex. If I then export the manual to 2.0.x format and then
reopen the exported file in the master branch, I get the following
! Package xcolor Error: Undefined color `Color'.
Is this finding interesting/useful?

Oh, and on this score: Can you tell why we get the error? One way to
localize it would be
to bisect on the format to which you are converting.

Doesn't seem to be related to roundtrips. The problem seems to be related
to the conversion of arguments. See attached example.

[Jürgen Spitzmüller]

Post by Enrico Forestieri
Doesn't seem to be related to roundtrips. The problem seems to be related
to the conversion of arguments. See attached example.

The attached fixes the thinko in the lyx2lyx method. However, I do not know
what this method actually is supposed to convert, so I cannot really test
if the conversion works. The respective commit (by Uwe) is this:
http://www.lyx.org/trac/changeset/b42604c7aa0551c4761605f1e9131b05522c98b5/lyxgit

BTW I still get a console message when opening with LyX 2.2 (but not so
when opening with LyX 2.1).

JÃŒrgen

Post by Enrico Forestieri
--
Enrico

[Richard Heck]

Post by Enrico Forestieri
Doesn't seem to be related to roundtrips. The problem seems to be related
to the conversion of arguments. See attached example.
The attached fixes the thinko in the lyx2lyx method. However, I do not
know what this method actually is supposed to convert, so I cannot
really test if the conversion works.

This is supposed to convert the arguments in the Initial style, from
initials.module.

Richard

[Enrico Forestieri]

Post by Jürgen Spitzmüller
The attached fixes the thinko in the lyx2lyx method.

However, if you insert any ERT in the Initials layout, the conversion
will still fail in the same manner. There is something wrong here.

Post by Jürgen Spitzmüller
BTW I still get a console message when opening with LyX 2.2 (but not so
when opening with LyX 2.1).

I get no console messages.

[Jürgen Spitzmüller]

Post by Enrico Forestieri

Post by Jürgen Spitzmüller
The attached fixes the thinko in the lyx2lyx method.

However, if you insert any ERT in the Initials layout, the conversion
will still fail in the same manner. There is something wrong here.

As I wrote, I do not understand what the method is supposed to do. When we
have argument insets for initials, why bother converting ERT?

Post by Enrico Forestieri

Post by Jürgen Spitzmüller
BTW I still get a console message when opening with LyX 2.2 (but not so
when opening with LyX 2.1).

I get no console messages.

LyX: \end_deeper: depth is already null [around line 2762 of file
/tmp/lyx_tmpdir.pyVFzUDL2492/Buffer_convertLyXFormatXXXXXX.lyx.uWhbNdtI2492
current token: '\end_deeper' context: 'InsetText::read']

JÃŒrgen

Post by Enrico Forestieri
--
Enrico

[Jürgen Spitzmüller]

Post by Jürgen Spitzmüller
LyX: \end_deeper: depth is already null [around line 2762 of file
/tmp/lyx_tmpdir.pyVFzUDL2492/Buffer_convertLyXFormatXXXXXX.lyx.uWhbNdtI2492
current token: '\end_deeper' context: 'InsetText::read']

False alarm. This was a particular beamer document (with separators).

JÃŒrgen

[Georg Baum]

Post by Jürgen Spitzmüller
As I wrote, I do not understand what the method is supposed to do. When we
have argument insets for initials, why bother converting ERT?

It is not only used for initials, but in a lot more places. It is supposed
to resolve hacks like an ERT "}{" or which Uwe used a lot before
InsetArgument supported position indices.

IMHO it was a mistake to add convert_TeX_brace_to_Argument() at all: It
works only for a restricted set of layouts which are used in the LyX
documentation. It does not work for user defined layouts which might need
the same hack in older versions. Furthermore, it is not a file format change
in the strict sense, but tries to get rid of some workarounds which were
needed in the old version. This can never work 100%, and IMHO we should
never try to interpret ERT.

However, now we have this beast, and it is too late to get rid of it. I
briefly looked at Richrads latest patch, and tested that it does not break
LyX documentation roundtrip, so I believe it could go in. However, if
possible it would be good if somebody could also test itz with some eother
layouts which use convert_TeX_brace_to_Argument(), e.g. from IEEEtran (I
don't have such files).


Georg

[Jürgen Spitzmüller]

Post by Georg Baum
IMHO we should
never try to interpret ERT.

In the beamer case, it was necessary, since old ERT arguments cease to work
due to the redefinitions.

JÃŒrgen

[Jürgen Spitzmüller]

Post by Jürgen Spitzmüller
The attached fixes the thinko in the lyx2lyx method. However, I do not
know what this method actually is supposed to convert, so I cannot really
http://www.lyx.org/trac/changeset/b42604c7aa0551c4761605f1e9131b05522c98b5/lyxgit

A closer look reveals that the convert_TeX_brace_to_Argument method itself
must be fixed. This method blindly searches for ERT insets from the
beginning line without considering whether the inset is in the current
paragraph. I suppose this will corrupt documents in many cases.

The conversion of Initial arguments probably fails due to the hard-wired
line-countings this method relies on.

I do not have time to fix any of this, though.

JÃŒrgen

[Richard Heck]

Post by Jürgen Spitzmüller
The attached fixes the thinko in the lyx2lyx method. However, I do
not know what this method actually is supposed to convert, so I
cannot really test if the conversion works. The respective commit
http://www.lyx.org/trac/changeset/b42604c7aa0551c4761605f1e9131b05522c98b5/lyxgit
A closer look reveals that the convert_TeX_brace_to_Argument method
itself must be fixed. This method blindly searches for ERT insets from
the beginning line without considering whether the inset is in the
current paragraph. I suppose this will corrupt documents in many cases.

This:


Todo: this routine can currently handle only one mandatory
argument of envi
'''
+ end_layout = find_end_of_layout(document.body, line)
lineERT = line
endn = line
loop = 1
- while lineERT != -1 and n < nmax + 1:
- lineERT = find_token(document.body, "\\begin_inset ERT", lineERT)
- if environment == False and lineERT != -1:
+ while n < nmax + 1:
+ lineERT = find_token(document.body, "\\begin_inset ERT", lineERT,
end_lay
+ if lineERT == -1:
+ break

will solve some of that problem. But I see what you mean about all the
hard-coding. I may just try to re-write the method. But without any
comments it is hard to know what is supposed to be happening.

Richard

[Jürgen Spitzmüller]

Todo: this routine can currently handle only one mandatory argument
of envi
'''
+ end_layout = find_end_of_layout(document.body, line)
lineERT = line
endn = line
loop = 1
- lineERT = find_token(document.body, "\\begin_inset ERT", lineERT)
+ lineERT = find_token(document.body, "\\begin_inset ERT", lineERT,
end_lay
+ break
will solve some of that problem.

Good. Note that there is a second search for ERT inset in that method
(check for "lineERT2"), which probably needs similar handling.

But I see what you mean about all the hard-coding. I may just try to

re-write the method. But without any comments it is hard to know what is
supposed to be happening.

That would be great. I think I understand a bit of it, so please ask if
things are unclear.

For the time being, the fix above should go in, I think.

JÃŒrgen

Richard

[Richard Heck]

Post by Richard Heck
Todo: this routine can currently handle only one mandatory
argument of envi
'''
+ end_layout = find_end_of_layout(document.body, line)
lineERT = line
endn = line
loop = 1
- lineERT = find_token(document.body, "\\begin_inset ERT", lineERT)
+ lineERT = find_token(document.body, "\\begin_inset ERT",
lineERT, end_lay
+ break
will solve some of that problem.
Good. Note that there is a second search for ERT inset in that method
(check for "lineERT2"), which probably needs similar handling.
But I see what you mean about all the hard-coding. I may just try
to re-write the method. But without any comments it is hard to
know what is supposed to be happening.
That would be great. I think I understand a bit of it, so please ask
if things are unclear.
For the time being, the fix above should go in, I think.

The attached puts limits on all the various searches. It also
re-organizes some of the code, as we don't need to hardcode quite so
many things. Can someone have a quick look before I commit.

Richard

[Jürgen Spitzmüller]

The attached puts limits on all the various searches. It also re-organizes
some of the code, as we don't need to hardcode quite so many things. Can
someone have a quick look before I commit.

The patch makes sense AFAICS.

JÃŒrgen

Richard