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

Improve error alignment when multiple errors occur. #355

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

ratmice
Copy link
Collaborator

@ratmice ratmice commented Oct 4, 2022

When rust prefixed the error text with "Error:", and multiple errors occur, indent them on their own line.
Would it be better to "\n\t", also in the single error cases as well?

When rust prefixed the error text with "Error:", and
multiple errors occur, indent them on their own line.
@ratmice ratmice requested a review from ltratt October 4, 2022 12:21
@ltratt
Copy link
Member

ltratt commented Oct 6, 2022

I must admit I'm not sure I can remember what outcome this PR refers to! What triggers this code execution path?

@ratmice
Copy link
Collaborator Author

ratmice commented Oct 6, 2022

This is the build.rs one for cargo build/CTBuilder::build(), the other paths all print errors themselves so they don't need the same treatment, since they aren't interspersed with IO errors and print errors themselves.

The following is the master output vs this branch output (last), from commenting out the yacc_flags in cttests/warnings.test

  --- stderr
  Error: Unused rule at line 5 column 1
  Unused token at line 2 column 8
  --- stderr
  Error:
        Unused rule at line 5 column 1
        Unused token at line 2 column 8

@ltratt
Copy link
Member

ltratt commented Oct 7, 2022

Got it!

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 7, 2022

Build succeeded:

@bors bors bot merged commit 2eeb903 into softdevteam:master Oct 7, 2022
@ratmice ratmice deleted the error_alignment branch October 9, 2022 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants