OCA / odoo-pre-commit-hooks

Linters of Odoo addons that complement pylint-odoo
GNU Affero General Public License v3.0
10 stars 12 forks source link

[IMP] Better error output #106

Closed ivantodorovich closed 1 month ago

ivantodorovich commented 2 months ago

Hey guys!

I'm playing a little bit with this, trying to get a better colored output.

What do you think of this? I kind of based it on ruff's styles

image

Besides the colors, I also sorted the errors by file and line, which is a more natural way of processing them for the user

antonag32 commented 2 months ago

I like the typing, defining an error class and a method to append errors looks like a step in the right direction. If we are sorting, can we also group? I think that could help us reduce the signal to noise ratio. If I recall correctly I think pylint groups by file (module) or error type, I don't remember which.

Also I would advise verifying if colored output still works when running as a pre-commit hook, since code here is mostly used like that.

moylop260 commented 2 months ago

Thank you for colorized! :)

I see the last section could be not needed since that adds a newline and it even doesn't have a structure for all cases

Notice the following case:

IMHO it could be the same line

The following output has a tuple format (..., ):

moylop260 commented 2 months ago

If we are sorting, can we also group?

IMHO it could be a different issue since that grouping by file it could remove the feature to use the shift + click in order to open the file in the exact line-column in vscode and so on.

Of course, it has solution if we group-by only by odoo module but we could discuss it from another issue IMHO

ivantodorovich commented 2 months ago

I see the last section could be not needed since that adds a newline and it even doesn't have a structure for all cases

🤔 the way I imagined it, that "info" section could even take multiple lines to display, for example, the locations of the duplicated xmlids. It's up to each check to decide what show there.

But, let's say it's restricted to 1 line, I'm not sure it looks any better:

image

Honestly, the scenario that looks better and makes it easier to read is the one we don't print any extra information at all:

image

Which is actually similar to what ruff does. I'm starting to be more inclined to this option What do you think?

moylop260 commented 1 month ago

@antonag32 @luisg123v @hugho-ad

Please, review