[PATCH] should we increase this magic number? (LaTeX log scanning code)

[Scott Kostyshak]

If you export ACM-sigplan.lyx in the GUI, LyX says it was exported
successfully and shows the PDF. But if you look at the terminal output
you will see that pdflatex exited with error. The reason LyX doesn't
pick this up is because (1) it does not check the exit code (which is
a separate bug in my opinion) and (2) it does not detect an error when
scanning the log file. This patch addresses (2). I attach the log file
in case you are interested but do not want to install the LaTeX files
needed to test for yourself.

The problem was that we only register an error if there is 10 or fewer
lines in-between the "!" line and the line where the line number where
the error occurred (the "l." line). This patch increases that number
to 15. Why 15? Because we used 10 for 14 years and there weren't many
problems, at least not that anyone noticed. 15 is just enough to
detect the error in this particular case.

The number 10 was introduced at a2c6689c to address the following

+ * src/LaTeX.C (scanLogFile): errors where the line number was not
+ given just after the '!'-line were ignored (from Dekel Tsur).

Am I correct that the only drawback is in theory performance?

I would really like to fix (1) above, so I'm OK if there are
objections to this patch, as long as we agree that (1) should be
fixed.

Scott

[Pavel Sanda]

+++ b/src/LaTeX.cpp
@@ -809,7 +809,11 @@ int LaTeX::scanLogFile(TeXErrors & terr)
if (!getline(ifs, tmp))
break;
tmp = rtrim(tmp, "\r");
- if (++count > 10)
+ // 15 is somewhat arbitrarily chosen, based on practice.
+ // We used 10 for 14 years and increased it to 15 when we
+ // saw one case. The trade-off of looking ahead too far
+ // is performance.

The performance difference seems to be negligible here. I would be rather worrier
whether we can't catch unrelated error... P

+ if (++count > 15)
break;
} while (!prefixIs(tmp, "l."));
if (prefixIs(tmp, "l.")) {

[Scott Kostyshak]

Post by Pavel Sanda

+++ b/src/LaTeX.cpp
@@ -809,7 +809,11 @@ int LaTeX::scanLogFile(TeXErrors & terr)
if (!getline(ifs, tmp))
break;
tmp = rtrim(tmp, "\r");
- if (++count > 10)
+ // 15 is somewhat arbitrarily chosen, based on practice.
+ // We used 10 for 14 years and increased it to 15 when we
+ // saw one case. The trade-off of looking ahead too far
+ // is performance.

The performance difference seems to be negligible here. I would be rather worrier
whether we can't catch unrelated error... P

I think see your point. For there to be a problem, there would have to
be two "! " lines before there is a "l." line (which let's say should
be associated with the second "! " line. We could then falsely
associate it with the first "!" line. I don't know enough about LaTeX
logs to have an intuition for whether that's possible. What do you
think about changing the while condition to the following?

while (!prefixIs(tmp, "l.") && !prefixIs(tmp, "! "))

Scott

Post by Pavel Sanda

+ if (++count > 15)
break;
} while (!prefixIs(tmp, "l."));
if (prefixIs(tmp, "l.")) {