Skip to content
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

Space out errors and warnings for readability #961

Merged
merged 2 commits into from
Mar 19, 2022
Merged

Conversation

sezna
Copy link
Contributor

@sezna sezna commented Mar 18, 2022

I am working on #308, and I'm noticing that when a lot of errors are generated, they can be very difficult to visually differentiate.

image

This PR adds (somewhat primitive) visual boundaries between the errors/warnings:

image

I don't know if this is the best way of doing this, and I'm open to feedback and/or using an alternate separator. But I do think this improves the UX a bit.

@sezna sezna added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels Mar 18, 2022
@sezna sezna self-assigned this Mar 18, 2022
@sezna
Copy link
Contributor Author

sezna commented Mar 18, 2022

I guess --> error is the separator already, but visually, it all blends together to me. I'm open to being overridden here, I suppose.

Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the lockfile updated?

@sezna
Copy link
Contributor Author

sezna commented Mar 18, 2022

I'm not sure what exactly changed, but I just reset the lockfile and rebuilt, and it got updated again. Maybe the toml changed on master but the lockfile wasn't committed?

@adlerjohn
Copy link
Contributor

Ah makes sense. Can you make a separate PR to update the lockfile in isolation?

Copy link
Contributor

@eureka-cpu eureka-cpu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, can't think of a better way off the top of my head.

@sezna sezna requested a review from adlerjohn March 18, 2022 20:57
Copy link
Contributor

@nfurfaro nfurfaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change would be helpful. A possible improvement might be to either:

  1. change the color of the ----
  2. use more ----'s ! ie: Each error in its own box. Not sure if that would look crowded though.

Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, I have no major preference either way :)

@sezna sezna merged commit 3dda3d2 into master Mar 19, 2022
@sezna sezna deleted the sezna/space-out-warnings branch March 19, 2022 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants