Reconsider output of CIF checks
From !416 (comment 1042190):
Let's just grab this for an error message layout discussion. A few random notes:
- Inserting the position between the group/element name and the actual error makes reading the error hard. Position info should be before or after the element + message I think.
g.A
line moved betweeng.a
andg.a.U
errors, splitting the latter.- Position information is jumping around in the file randomly which makes it quite useless for fixing errors. Eg "spec" error is at the bottom.
- Position information has quite some clutter, eg
"common_checks/EqnNotAllowedCheck.cif" at 19, 7
would be enough (possiblyline
instead ofat
).Jumping around in the file was already happening of course, it's just more visible now. As we haven't used this form of reporting in practice much I am not sure how serious it is.
For single files you'd definitely want to sort on position I think, not sure what would be useful if you include files. (My gut feeling says top-down from the root file in inclusion order, but that information may be lost or hard to retrieve.)
To reduce the line length, maybe the filename can be at a separate line and all errors in it below it with the line and column information? I would put the position near the start, it's mostly equally long everywhere, you can easily skip it and read the errors messages vertically only.
I am not sure how much can be done here.
For inspiration an error message from a C++ compiler where I added
f
at a random line of the source:/home/.../Src/gfx.cpp: In member function ‘bool reader::read_string(std::string*)’: /home/.../Src/gfx.cpp:128:16: error: ‘f’ was not declared in this scope 128 | size_t idx;f | ^ /home/.../Src/gfx.cpp:129:39: error: expected ‘;’ before ‘)’ token 129 | for (idx = 0; idx < iLength; idx++) { | ^ | ;
This is way more than we can do currently but it looks good :)