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

Apply indent in causes on each new line #54

Merged
merged 16 commits into from
Dec 24, 2019
Merged

Conversation

yaahc
Copy link
Contributor

@yaahc yaahc commented Dec 11, 2019

No description provided.

@yaahc

This comment has been minimized.

@yaahc

This comment has been minimized.

@yaahc

This comment has been minimized.

@yaahc

This comment has been minimized.

Prior to this change, the debug format always printed 4 spaces and then
the index of the error in the chain. Previous changes made it so that in
the cause chain we indent each new line to line up with the first line.
Together these two changes mean that if the index is longer than 1
character the first line will become misaligned. This was fixed by using
a left pad to 4 characters so the : is always on the 5th character and
all lines line up consistently.
@yaahc

This comment has been minimized.

@yaahc

This comment has been minimized.

@dtolnay
Copy link
Owner

dtolnay commented Dec 11, 2019

I think I would prefer to keep the first error on the same line as the "Error:" printed by main.


I'd have to figure out a way to either format differently if I'm being called from anyhow vs main (maybe some method of the Formatter?)

Maybe you would be interested in writing an RFC for https://www.reddit.com/r/rust/comments/dzq42h/error_handling_survey/f89icon/ ?

@yaahc
Copy link
Contributor Author

yaahc commented Dec 11, 2019

Fuck yes I'd be interested. If I'm removing the newline for the error, can I leave in the special casing for handling whitespace in source errors?

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks! This is great.

I am merging this with a few minor changes noted below.

src/fmt.rs Show resolved Hide resolved
src/fmt.rs Show resolved Hide resolved
@dtolnay dtolnay merged commit ba0f8c9 into dtolnay:master Dec 24, 2019
@dtolnay
Copy link
Owner

dtolnay commented Dec 24, 2019

Published in 1.0.26.

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