There are still a few warnings of the kind
(style) Variable 'x' is assigned a value that is never used.
since I did not touch code where I was not sure whether there might be a real
bug, and I kept some for symmetry reasons as well.
These were all flagged by "(style) The scope of the variable 'x' can be reduced."
Narowing the scope improves readability, and if it is in a loop then the
compiler will be clever enough to produce efficient code, we do not need
manual optimization for POD types.
As Günter found out running the tex2lyx tests overwrites the test references
if the build directory is identical to the source directory. Therefore the
tests would always pass, but git diff would show a non-empty diff if the tests
should have failed. Since it is better anyway to build in a separate directory
we simply do not support srcdir = builddir for the tests and abort with an
error.
This brings the external inset on par with the graphics insets as far as the
clipping option is concerned. The graphicxs package supports both: A bounding
box without units (which means that bp ia assumed), and a bounding box with
units, so we can simply output the values including the units.
Being able to compile document with zipped .eps files was a useful feature of
the graphicxs package 20 years ago, but the LyX support is no longer relevant:
- The flag is ignored if preview is on
- If pdflatex is used then uncompressing happens during the compilation anyway
- If set, the flag prevents LyX from issuing proper error messages if
something with the image is wrong
- For hard disk capacities from 20 years ago not uncompressing is a useful
feature, but for current hard disk capacities it does not matter
- The external inset does not have it, and if we want to merge both insets
one day we would need to implement it there, which is even more difficult
than in InsetGraphics
Unfortunately I overlooked in 44f73b0650 that the first three whitespace
changes in box-color-size-space-align.lyx.lyx were actually correct, so they
should not have been reverted. In detail:
1), 2): The space after \raggedleft must not be part of the ERT inset, but it
is ouput by check_space() as part of the standard text which follows.
3): The space in front of www is caused by the fact that there is a
newline between the opening brace of the parbox and the \centering
command, so this space is not the one after \centering (which is
correctly swallowed). This additional space is in fact not needed,
and the contents would look better in LyX without it, but since it is
not caused by special code I'll put it back in the refernce for now.
We can still improve this in the future if anybody has a good idea.
The remaining whitespace issues are all fixed by a simple change in
parse_text(): Instead of always eating whitespace after detecting \centering
et al, and always output a space as part of the ERT if these commands need an
ERT, let the standard space handling mechanism kick in: skip whitespace if
no ERT is used (in this case LyX will always output the needed space), and
do not touch whitespace if an ERT is used.
These should not have been done without discussion.
- Removal of the dcolumn table in 1a8b74f5e1. Even if LyX does not support
dcolumn anymore, it is still a useful test whether tex2lyx imports it
correctly.
- Removal of the first "%% LyX" line. A long time ago it was decided (after
long discussion between at least Jean-Marc, Uwe and me), that this line is
interpreted by tex2lyx, and used to remove some LyX-generated preamble code.
These lines in the current tests exist on purpose (one can see in the diff
how the removal added unwanted stuff). I do not really like the
interpretation of the "%% LyX" line, but if this behaviour is to be changed
then this needs discussion first.
- Changed comment of \date. The comment was put there on purpose, and the
warning which was "fixed" by the change hints at a limitation in LyX, not a
tex2lyx problem (LyX does not know that a comment inset between some title
insets is OK). The roundtrip .tex output was OK with the old version.
- Change of \verbatiminput{foo}. This was supposed to test whether a
verbatim inset is correctly created even if the included file does not
exist.
- Removal of \lyxlines. Although these tests test input of files created by
old LyX versions, they are useful.
- Change of the lemma in test-modules.tex. The old version was put there on
purpose, and the file itself explains why it is translated to ERT.
There is a general problem of tex2lyx handling theorems. it is not sufficient to cure only one instance as I did. For more info see bug #9561.
Update the references accordingly.
With the now removed command we said that this is a file created by LyX but this is not the case
- also replace a comment to avoid LaTeX warnings about mixing title and non-title stuff
before LyX 2.1 was released the dcolumn support was dropped and another method was used to align at the decimal point. Nevertheless the old LaTeX table was even wrong in terms of dcolumn
Now a table is used that uses the decimal alignment as it is supported by LyX
The idea is to get a compilable file that does not require programs that are only available on certain platforms.
For example on Windows there is no Gnumeric available (only a very outdated and unsupported version with bugs).
there were 2 issues:
- the body was not output to the Preamble
- the theorem module already defines a theorem. It must not be output to the preamble to avoid a redefinition error of LaTeX
update the test files accordingly
\framebox{} is equal to \fbox{} and \makebox{} is equal to \mbox{}
When I once wrote this code LyX did not support \fbbox and \mbox and therefore had to use a workaround.