On Wed, Jun 05, 2019 at 07:50:55PM -0600, Joel Kulesza wrote: > Thanks for putting these together so quickly. Some comments: > > 1. p1: The notation seems inconsistent, referring to both citations and > references. From what I understand, I would refer strictly to citations > (perhaps preceded by bibliography, to help orient a reader).
I tried to disentangle little bit, see below. > 2. p1: I believe that when a citation key cannot be found, the key is > given. Could this be provided to the user to help him/her better locate > the issue? Looks little bit difficult within the current machinery (if I understand it correctly). Cits can be fixed by incremental runs and we see that in updated result value of scanres of LaTeX::run, i.e. if fixed it does not contain UNDEF_CIT anymore. The accompanying structure of errors "terr" does not get reset, so once you push there some info about missing citation it's not going away. At the end if you by re-runing fix citation problem but there still remain some other problem user will wrongly see it within the list of errors... I actually do not understand why not reseting "ter" before each run. Seems reasonable to me, but it's hard to test without having examples which produced all the trickery in the code. Otherwise we could just grep in scanLog for the label/citation name and push it to the error dialog. > 3. p2: Perhaps "undefined cross-references"? Same comment about echoing > the key that is incorrectly cross-referenced? We are getting this error from LaTeX and it says "reference" in both cases when cit or cross-ref is missing (and maybe in some different cases I do not know yet). So I kept it intentionally vague, I changed what we grep for so it's hopefully better now. > To your concern about being too aggressive: this is potentially an issue. > However, while I find it a nuisance when I insert a bibliography, haven't > yet inserted citations, and have to dismiss the errors and "show anyway," I > prefer that situation to silent failures that I *don't* expect. The same > applies here. Why I am not particularly confident about the reference part is that some packages like hyperref might be doing some business with references in backgrounds (for building ToC or similar) and would flush similar warnings which are actually harmless in final pdf -- but we will bombard users with error dialog all the time. We can try to push to master and wait how much will people scream... > Do you have any thoughts on coloring the buttons in the GUI to indicate > breakage? Not enough visibile IMHO, and our current approach seem to be good enough. Updated patch is below. Pavel
diff --git a/src/Converter.cpp b/src/Converter.cpp index 8c22b1c42a..47ac15cb1c 100644 --- a/src/Converter.cpp +++ b/src/Converter.cpp @@ -881,6 +881,16 @@ Converters::RetVal Converters::runLaTeX(Buffer const & buffer, string const & co if (result & LaTeX::ERRORS) buffer.bufferErrors(terr, errorList); + if (result & LaTeX::UNDEF_CIT) + errorList.push_back(ErrorItem(_("Undefined citation"), + _("Undefined citation was found during the build, please check the Log."), + &buffer));; + + if (result & LaTeX::UNDEF_REF) + errorList.push_back(ErrorItem(_("Undefined reference"), + _("Undefined (cross)reference was found during the build, please check the Log."), + &buffer));; + if (!errorList.empty()) { // We will show the LaTeX Errors GUI later which contains // specific error messages so it would be repetitive to give @@ -909,6 +919,8 @@ Converters::RetVal Converters::runLaTeX(Buffer const & buffer, string const & co int const ERROR_MASK = LaTeX::NO_LOGFILE | LaTeX::ERRORS | + LaTeX::UNDEF_CIT | + LaTeX::UNDEF_REF | LaTeX::NO_OUTPUT; return (result & ERROR_MASK) == 0 ? SUCCESS : FAILURE; diff --git a/src/LaTeX.cpp b/src/LaTeX.cpp index 0144e1cf16..2c1e25a939 100644 --- a/src/LaTeX.cpp +++ b/src/LaTeX.cpp @@ -775,10 +775,16 @@ int LaTeX::scanLogFile(TeXErrors & terr) && contains(token, "on page") && contains(token, "undefined")) { retval |= UNDEF_CIT; + //"There were undefined references." - Global LaTeX error which could + //be used for both two warnings below } else if (contains(token, "Citation") && contains(token, "on input line") && contains(token, "undefined")) { retval |= UNDEF_CIT; + } else if (contains(token, "Reference") + && contains(token, "undefined") + && contains(token, "on input line")) { + retval |= UNDEF_REF; } } else if (prefixIs(token, "Package")) { // Package warnings