Capture warnings into diagnostic objects #625
Conversation
|
I really like the look of this! I think you're right that it will be helpful to use a small structure to allow for the capture of line-number information. (The line-numbering option is turned on in Tectonic by default, but I have to admit that I feel like I can never understand what it reports anyway.) |
|
The current Travis failure is due to formatting — our CI requires that (I'm not sure why Azure hasn't run — maybe it doesn't run for draft PRs? Or sometimes there's just a hiccup in the system.) |
Codecov Report
@@ Coverage Diff @@
## master #625 +/- ##
==========================================
+ Coverage 46.07% 46.13% +0.05%
==========================================
Files 138 138
Lines 59884 59956 +72
==========================================
+ Hits 27592 27660 +68
- Misses 32292 32296 +4
Continue to review full report at Codecov.
|
|
For some reason, codecov seems to be stuck, but CI is looking green otherwise. Is this ready for review? |
|
I've only added the diagnostics to a portion of the places where warnings or errors get emitted - and there's quite a lot of these. Should I do that in this PR or separate those into a new one? Either way I'll clean up the commits before mark this as ready for review. |
|
@ralismark I think it makes sense to have this PR work on the foundation without trying to cover every single message in the codebase. If nothing else, that leaves some nice low-hanging-fruit tasks for new contributors. |
The idea is that a Diagnostic is essentially a "builder" for a warning/error - we apply changes to it (e.g. adding strings) and eventually output it with diag_finish. Doing it this way allows us to add more information to warnings later, such as the filename & line number. Having individual diagnostic_t objects instead of a global warning allows multiple warnings to be constructed simultaneously (e.g. across multiple functions).
This allows functions to "tee" printed output into warnings. This makes it easier to support this new warning style.
ralismark
changed the title
|
This PR should be ready for review & merging now. |
Centralize the code that prints the file/line information and take more advantage of the `current_diagnostic` state -- automatically calling `ttstub_diag_finish()` when stopping a capture, and having the helper functions call `capture_to_diagnostic()` automatically.
…insert_dollar_sign()
Oops -- need to do this to tidy up Cargo.lock changes.
|
Thanks, this is fantastic! I've pushed a few changes to tidy up the API a little bit. Now, the warning messages captured into Tectonic will include file and line information unconditionally. I've also wired up the error reporting code in Presuming those all work, I'll merge this. (Looks like the "semistatic" macOS build is failing due to |
Namely, install pkg-config with brew. We didn't need to do this before, so maybe this error reflects a transient vcpkg issue, but it's easy enough to work around. (Hopefully.)
This is meant to resolve #577. The general design of this is have each warning/error be reported via a
Diagnosticobject (diagnostic_ton the C side) -- doing this will allow extra information to be added later (e.g. location of error). I've also added a sort of hack inxetex-output.cto be able to capture print messages into a diagnostic. The main thing left now is to wrap all the places where warnings/errors are emitted withand