flycheck / flycheck

On the fly syntax checking for GNU Emacs
https://www.flycheck.org
GNU General Public License v3.0
2.4k stars 449 forks source link

Identifying the main error in a group of related errors #1732

Open cpitclaudel opened 4 years ago

cpitclaudel commented 4 years ago

When we introduced the notion of related errors (#1249), we made the 'related' field hold an arbitrary symbol. But we didn't include a way to point to the "main/canonical" error. This makes it tricky to group errors so that related spans follow the main error (I'm thinking of the error list, for example).

Should we add a mechanism for this? We could just decide, by convention, that errors have their :group set to their parent error, for example (I think this would be compatible with the previous semantics).

We could also decide that it's a cons of (parent . children), or something similar. But I think we ought to introduce some way to tell which error in each group is the main one, if any.

fmdkdd commented 4 years ago

But I think we ought to introduce some way to tell which error in each group is the main one, if any.

I agree that in most examples I've seen, there is always a "main" cause of error, followed by additional "related" diagnostics.

Grouping related errors in the error list sounds like a great idea.

Short of adding yet another property on the error struct, main errors could also have a different :group value, e.g. a cons ('main . group-id). Whatever solution we pick, it shouldn't writing the integration tests too inconvenient.

cpitclaudel commented 4 years ago

What about allowing arbitrary nesting, then? 'group would contain a cons of (parent . children); parent would be nil for top-level errors. And this would make getting the related errors much faster (since we can get them by reading the field now). It's a breaking change, but I haven't seen much code that relies on this field on github, except flycheck-inline.

The next steps would be to add a flag to say whether related errors should always be shown.

Finally, we'd need to decide whether errors that appear in an error's group should also appear in flycheck-current-errors or not. I'm inclined to say no. WDYT?