Skip to content

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 between g.a and g.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 (possibly line instead of at).

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 :)