Lonami / pyndustric

A Python compiler for Mindustry's logic processors assembly language
MIT License
49 stars 9 forks source link

Improve errors #72

Closed smartycope closed 2 months ago

smartycope commented 2 months ago

As I was developing a program using pyndustric, I kept getting errors which were hard to read or with insufficient information. This update adds some information, makes error messages easier to read, and allows for custom descriptions to CompilerErrors.

smartycope commented 2 months ago

I figured it would be more Pythonic. Since the error codes (E003, etc.) are arbitrary, why not make them arbitrary in a way that is a little more useful? In addition, there's several instances where more description of the error would be useful, as opposed to a standard error description.

Lonami commented 2 months ago

I figured it would be more Pythonic. Since the error codes (E003, etc.) are arbitrary

Only partially arbitrary. The prefix indicates it's an error, and the rest is an increasing unique ID. I don't remember my rationale, but I was probably inspired by Rust (see https://doc.rust-lang.org/error_codes/error-index.html and https://rustwiki.org/en/error-index).

Seems stuff like ruffs has both codes and friendly names https://docs.astral.sh/ruff/rules. But I'm not sure how much that matters for us, since none of the errors can be disabled or turned into warnings.

there's several instances where more description of the error would be useful

No question here, but providing useful feedback isn't easy, so I haven't really prioritised that. I figured the code and position where the error occured might've been enough information. Along with a generic explanation of the problem of course.

Also, I made no promises of the error codes being stable, and I doubt anyone would care if we changed them right now, but it still feels wrong to straight up replace all of them. I don't know. WDYT?

smartycope commented 2 months ago

You bring up a good point. Tools like black, flake8, and compilers, and other formatters tend to identify errors by arbitrary codes, but I figured they do because it makes it easier to specify specific errors via the command line. We're not doing that at all though.

I wouldn't think replacing them would be a big deal, because they're only used internally. Nothing really changes from the user's perspective.