-
Notifications
You must be signed in to change notification settings - Fork 113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature: improve error messages for package field validation checks #994
Comments
Having slept on this, I think the paths to files should be relative to the package since that is where you are likely to be and so would be able to |
To add a +1 to the problem of identifying which field is the offending one (it's AST position is printed, instead of trying to parse a possibly valid name), I am sorting through a large list of errors (200+) and my output looks like this:
sorting through this part repeatedly is challenging to identify what to update:
|
Another example of where this could be improved is in the context for syntax failures. Recently I was working on a package and due to a edit error, I was told
This doesn't tell me which file is problematic (there are two manifest files and three fields definition files that could arguably be considered to be the object of the error) and does not show a context. Looking at the files I finally found the error (a single space addition, which was why it was hard to find).
Note that the error message points to line 34, but the actual error is on line 42. The disparity appears to be due to the error pointing to the last line of the last valid list element. |
I have recently been doing a large update of packages in integrations and have found in that process that the errors that are printed could be improved.
Two example cases:
In both cases there is unnecessary information that reduces the readability of the error, particularly when there are many lines of errors. So I would suggest that the file paths be made relative to the package at least, and probably to the data stream.
In the case of the invalid field, the value that is invalid is not displayed, only the values that it may take. This in conjunction with the fact that the name of the field is not printed, only its position in the AST, make it difficult to find the offending value. In the case above it is not particularly hard since it is early in the file, but some are quite deep. So I would suggest that the offending value be printed, the name of the field (i.e. the value of the name field in the yml) and the file position also be provided (e.g. audit/fields/fields.yml:21:13 in the case above) to allow direct movement to the error in an editor.
The impact of the current output format can be clearly demonstrated with the following detail.
This would be made much clearer if the path were made relative to the data stream, file positions were emitted and the outputs were made to be consistent between runs (preferably grouped by file in sections in lexical order of datastream name then file name grouped and then by position in file). A sketch of how this might look is in the following detail.
It is easier to read, follows the ordering of the files in an editor's file pane and the fields are presented in the order that you would find them in the files as they are being edited (not done in the example here but is straighforward to do mechanically).
The text was updated successfully, but these errors were encountered: