PR: Enhance the display of warnings and errors

[goanpeca]

Description of Changes

• Wrote at least one-line docstrings (for any new functions)
• Added unit test(s) covering the changes (if testable)

• Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #9247

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @goanpeca

[pep8speaks]

Hello @goanpeca! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file spyder/plugins/editor/panels/linenumber.py:

Line 118:80: E501 line too long (80 > 79 characters)

Comment last updated at 2019-05-25 02:41:47 UTC

[goanpeca]

@ccordoba12 currently the line number area is not changing size when zooming in/out. I think it perhaps should?

[CAM-Gerlach]

@goanpeca I cannot properly test this due to #9425 , which I've repro'd on multiple branches consistently. However, some initial comments:

• Could you capitalize the first letter in the Pyflakes message, so it matches the others and doesn't look out of place?
• If Pyflakes doesn't have error codes, can you just make the parenthetical be (pyflakes) s (or (pyflakes error)/(pyflakes warning) if there are multiple types) since there's no reason to show the whole thing when there's no error code?
• Code is redundant and appears in every message, taking up space; can you just remove it?

Also, how do you enable those pylint errors? They don't show up, even after running static code analysis.

[goanpeca]

Thanks for the review @CAM-Gerlach

Could you capitalize the first letter in the Pyflakes message, so it matches the others and doesn't look out of place?

Yep

If Pyflakes doesn't have error codes, can you just make the parenthetical be (pyflakes) s (or (pyflakes error)/(pyflakes warning) if there are multiple types) since there's no reason to show the whole thing when there's no error code?

They do have error codes http://flake8.pycqa.org/en/2.5.5/warnings.html

Code is redundant and appears in every message, taking up space; can you just remove it?

Not sure, @ccordoba12 wanted it to be obvious so I will defer to his judgement.

Also, how do you enable those pylint errors? They don't show up, even after running static code analysis.

You have to use the latest pyls (but spyder will complain you need a lower version)

[goanpeca]

[CAM-Gerlach]

I assume you're aware, but just in case, pyflakes messages still have the spurious E in your screenshot.

[goanpeca]

I assume you're aware, but just in case, pyflakes messages still have the spurious E in your screenshot.

http://flake8.pycqa.org/en/2.5.5/warnings.html

[CAM-Gerlach]

@goanpeca Okay, but I'm a little confused that's flake8, not vanilla pyflakes, which it explicitly says on that page does not provide error code (and none are provided in your screenshot).

[goanpeca]

@CAM-Gerlach you are right, my bad!

[goanpeca]

[ccordoba12]

Thanks @goanpeca for this! Things look really nice!!

I have a couple of quick comments:

1. Instead of using the information sign for hints, please use mdi-lightbulb to follow VSCode lead regarding that symbol.
2. Please use a nice shade of green instead of blue for hints, because blue is already used by TODO's.

[ccordoba12]

Code is redundant and appears in every message, taking up space; can you just remove it?

I disagree. In six years of maintaining Spyder nobody has complained about it. Besides, it's much better to be explicit than a bit vague about those messages too. So in my role as BDFL of Spyder I say that stays.

[goanpeca]

Please use a nice shade of green instead of blue for hints, because blue is already used by TODO's.

Ok, I will post a screenshot to see how it looks

[goanpeca]

I disagree. In six years of maintaining Spyder nobody has complained about it. Besides, it's much better to be explicit than a bit vague about those messages too. So in my role as BDFL of Spyder I say that stays.

@ccordoba12, I was referring to the word code in messages I added (see first screenshot see the word code in parentheses), not about the title.

[ccordoba12]

I was referring to the word code in messages I added

Ok, sorry. Perhaps they should stay? I mean it's hard to understand what those messages mean without the word code on them.

[goanpeca]

@ccordoba12

[CAM-Gerlach]

If you want to keep "code" repeated in every message for the sake of explicitness, its not a big deal, though I'd prefer it without it since the user can reasonably infer what it means (at worst, they have to google it their first time, which is exactly what they likely want to do anyway).

However, I do strongly disagree with the green color for information. Blue has long been the standard color for this across all the UIs I'm familiar with, while Green means "Good"/"Ok", which is not the case here. For example, the first four relevant results for errors warnings information colors on Google state that the universal convention is red-error, orange/yellow-warning, blue-info, and green-ok/success/good. If you feel it conflicts unduly with the annotation color, then that color should be changed to something else, like grey.

[bcolsen]

I really like this change!

However, I do strongly disagree with the green color for information.

I would go back to the blue for the info.

Ok, sorry. Perhaps they should stay? I mean it's hard to understand what those messages mean without the word code on them.

I think pyflake: E0602 would look good and still googles well. I think the word "code" in there is a little distracting.

I also think that the error codes are in general a little distracting and not too important on first glance or for new users. Can we make the error stand out from the error code with a different brightness or maybe color. For example have the (pyflake: E0602) part a darker grey or make the other part lighter.

[ccordoba12]

I would go back to the blue for the info.

We have a lot of things that are blue in the editor panels: the TODO's symbol (as I said before) and the debugger arrow. So if you want blue for info, we really need to change to green those other symbols. @goanpeca, please do that as part of this PR, but you also need to change those colors in ScrollFlagArea.

I think pyflake: E0602 would look good and still googles well. I think the word "code" in there is a little distracting

Ok, fair enough. I agree with this suggestion.

For example have the (pyflake: E0602) part a darker grey or make the other part lighter

This is a good suggestion too. I think the error code should be lighter to make it less relevant than the rest of the text.

[CAM-Gerlach]

We have a lot of things that are blue in the editor panels: the TODO's symbol (as I said before) and the debugger arrow.

TODOs/etc can be grey or (if not visible enough) purple. Debugger error can be green.

I think pyflake: E0602 would look good

Agreed as well (though to be really nitpicky, its pyflakes and its the one that doesn't have error codes :)

I think the error code should be lighter to make it less relevant than the rest of the text.

I agree with having it a different shade, but in our default dark theme, having it lighter would make it much more visible/prominent, not less. If we wanted something that would work across our themes, we could just use italic instead (though that might not look quite as clean)?

[goanpeca]

Thoughts @bcolsen @ccordoba12 ?

[goanpeca]

We will still need to go over all the colors in Spyder at some point before the official release of 4.0 so I suggest we do not take more time than it is needed to get this functionality going.

We should be moving faster and leave all the color and style discussions for another issue/PR and focus on the actual tooltip (what this PR is about) and not on the icons and colors on the LineNumber/ScrollBar area.

[goanpeca]

On another note, do we plan to add additional highlights to the code editor words to indicate the problems (errors, warning, info, hint) on the text itself, using curly underline and colors?

[ccordoba12]

Thoughts @bcolsen @ccordoba12 ?

Beautiful!! I really like it! I was about to suggest using magenta for TODOs, so that's really cool.

We should be moving faster and leave all the color and style discussions for another issue/PR and focus on the actual tooltip (what this PR is about) and not on the icons and colors on the LineNumber/ScrollBar area.

Ok, agreed. Please open a new issue about it so we don't forget to address that, either in beta3 or beta4.

We should do two things about that: define a central place to import different colors and (in my opinion) change the debugger arrow to green.

On another note, do we plan to add additional highlights to the code editor words to indicate the problems (errors, warning, info, hint) on the text itself, using curly underline and colors?

Yes, I was also about to suggest that. Please open another issue about it.