Uwe Stöhr wrote:
> Am 31.05.2015 um 22:01 schrieb Georg Baum:
>
>> I tested that the old LyX file produced correct .tex results, now I see
>> that it creates correct .tex results with the new version as well. It did
>> not occur to me that both hor_pos "c" and hor_pos "l" in .lyx would
>> produce the same .tex output. Why is that the case? This looks like a bug
>> in LyX to me.
>
> I cannot reproduce this. Attached is a lyX example file showing that the
> hor_pos is correctly treated.
If you apply the hor_pos changes in my first patch (which I did not apply to
master), then the .tex files generated from these changed .lyx files do not
change. This is what I do not understand.
>>> This might be an issue:
>>> The TeX code is
>>> {\fboxsep 35pt\shadowsize 15pt\shadowbox{\centering www}}
>>> and tex2lyx correctly imports "\centering" as TeX code. The TeX code
>>> inset is correctly followed by a space. It would be better to include
>>> the space to the TeX code inset. I will have a look at this after your
>>> merge.
>>
>> This is the problem. There was no space issue to fix, the old code was
>> correct.
>
> But there was no old code.
There was old code: Previously, these commands were handled by the standard
mechanism for unknown commands. This mechansim does not add explicit spaces
behind commands, it relies on check_space() to do the correct thing.
> I added support for \raggedleft/right and
> \centering to tex2lyx in commit
> 1ace9f5ac267e77c7c7fe4bf9b659b6fe7e297fb
> I also updated the reference correctly.
>
> > Please remember that any space after a command such as \centering
>> is eaten, therefore we did the same in tex2lyx, and we should do it
>> again.
>
> In this case the space must not be eaten. Otherwise
> \shadowbox{\centering www}
> would lead to
> \shadowbox{\centeringwww}
The standard mechanism to handle these situations is check_space(). This
should be used here as well.
> With the current code in tex2lyx one correctly gets
> \shadowbox{\centering www}
> Therefore I don't understand why you reverted by changes in the
> reference .tex files. I added support for \raggedleft etc. and therefore
> of course also needs to update the reference.
Yes, but the reference should only be updated for changes that fall into the
following two categories:
1) a new test, which was added to test the new feature.
2) an old test, which was previously output using ERT, and which is now
output as native LyX inset.
It should _not_ be updated for changes that fall into the following
category:
3) an old test which produces now different output as a side effect of the
new code, but does not produce the native inset which was newly supported by
to tex2lyx.
I only reverted reference changes of this last category.
> As it currently is i get test failed but the resulting LyX file is
> exactly as it should, only the .lyx.lyx file in git master is not.
No, the resulting LyX file is not exactly as it should. The attached change
improves the situation, with this one you only get three differences to the
test reference. I might have a further look in a few days, if I'll find some
time.
>>>> - Change of \verbatiminput{foo}. This was supposed to test whether a
>>>> verbatim inset is correctly created even if the included file does not
>>>> exist.
>>>
>>> As I said this is OK. I therefore created a new file without this to get
>>> a compilable result.
>>
>> Then you should not have changed the old file.
>
> But I did not. I just looked at the logs and cannot find a commit where
> I changed this.
eb90d531bed8cb732515c1ae10f43dfc3bbbb485
>>>> - Removal of \lyxlines. Although these tests test input of files
>>>> created by old LyX versions, they are useful.
>>>
>>> Useful for what?
>>
>> For testing that the different lyxline variants do not break.
>>
>>> Why should we keep stuff that has gone years ago?
>>
>> Because people might want to import old documents, and keeping the tests
>> does not cost any effort.
>
> This is your philosophy. It is OK that we have different opinions about
> this but why do you make the rules? After some years it might not harm
> to kick out old stuff like this. Why am I not allowed to do this but you
> are allowed to revert my kick-out?
Are you joking? You kicked out my test without discussing it first, and then
you ask why I revert that change _after_ giving you the chance to explain
why you removed it?
>>>> - 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.
>>>
>>> Please look at the change I made. I only removed a superfluous
>>> \theoremstyle{plain
>>> from test-modules.tex
>>
>> This is not true, I did look at the change you made.
>
> This was my change in commit
> 041a3add6c044132ef42e2f7be77dc63214159ef
> I changed
>
> \theoremstyle{plain}
> \theoremstyle{plain}
> \newtheorem{thm}{\protect\theoremname}
> \theoremstyle{plain}
> \newtheorem{lem}[thm]{Lemma}
>
> to
>
> \theoremstyle{plain}
> \newtheorem{thm}{\protect\theoremname}
> \theoremstyle{plain}
> \newtheorem{lem}[thm]{Lemma}
The double theoremstyle was part of the removal, but you changed more. Look
at the diff of 041a3add6c044132ef42e2f7be77dc63214159ef.
> What is wrong with this?
The translation of a theorem to ERT was not tested anymore with the version
you put in at 041a3add6c044132ef42e2f7be77dc63214159ef.
Georgdiff --git a/src/tex2lyx/test/box-color-size-space-align.lyx.lyx b/src/tex2lyx/test/box-color-size-space-align.lyx.lyx
index 897859e..70a523c 100644
--- a/src/tex2lyx/test/box-color-size-space-align.lyx.lyx
+++ b/src/tex2lyx/test/box-color-size-space-align.lyx.lyx
@@ -1350,12 +1350,12 @@ status collapsed
\begin_layout Plain Layout
\backslash
-raggedleft
+raggedleft
\end_layout
\end_inset
-
+
\begin_inset Box Frameless
position "t"
hor_pos "c"
@@ -1376,7 +1376,7 @@ status open
\begin_layout Plain Layout
-www
+ www
\end_layout
\end_inset
diff --git a/src/tex2lyx/text.cpp b/src/tex2lyx/text.cpp
index d510c00..6800c51 100644
--- a/src/tex2lyx/text.cpp
+++ b/src/tex2lyx/text.cpp
@@ -4289,7 +4289,6 @@ void parse_text(Parser & p, ostream & os, unsigned flags, bool outer,
|| t.cs() == "shadowsize"
|| t.cs() == "raggedleft" || t.cs() == "centering"
|| t.cs() == "raggedright") {
- p.skip_spaces(true);
if (t.cs() == "fboxrule")
fboxrule = "";
if (t.cs() == "fboxsep")
@@ -4298,6 +4297,7 @@ void parse_text(Parser & p, ostream & os, unsigned flags, bool outer,
shadow_size = "";
if (t.cs() != "raggedleft" && t.cs() != "centering"
&& t.cs() != "raggedright") {
+ p.skip_spaces(true);
while (p.good() && p.next_token().cat() != catSpace
&& p.next_token().cat() != catNewline
&& p.next_token().cat() != catEscape) {
@@ -4310,8 +4310,10 @@ void parse_text(Parser & p, ostream & os, unsigned flags, bool outer,
}
} else {
// we only handle them if they are in a box
- if (!wasBoxAlign)
- output_ert_inset(os, '\\' + t.cs() + ' ', context);
+ if (wasBoxAlign)
+ p.skip_spaces(true);
+ else
+ output_ert_inset(os, t.asInput(), context);
}
wasBoxAlign = false;
}
