Return Result from Shape methods

rust-lang / rustfmt

Format Rust code

https://rust-lang.github.io/rustfmt/

Apache License 2.0

6.05k stars 888 forks source link

Return Result from Shape methods #6398

Open camsteffen opened 1 week ago

camsteffen commented 1 week ago

The Shape modifier methods now return Result<Shape, ExeedsMaxWidthError>
There are _opt variants of the methods for when an Option is still needed
- I believe this will remain useful in the long term, since sometimes we want to fallback and never error out
There is a minor change in behavior for when multiple methods are chained together - the width used in the error will be the last successfully modified width. I think this is okay?

ding-young commented 6 days ago

IMO, using the last successfully modified width is fine. Actually, that might be the more accurate width in some cases.

calebcartwright commented 5 days ago

IMO, using the last successfully modified width is fine. Actually, that might be the more accurate width in some cases.

by the way @ding-young thank you for all your excellent work this year! I know you worked closely with @ytmimi and we didn't get a chance to interact much, but i really appreciate all the work you did and I'm glad that you have expressed an interest in remaining engaged with it

calebcartwright commented 5 days ago

There is a minor change in behavior for when multiple methods are chained together - the width used in the error will be the last successfully modified width. I think this is okay?

@camsteffen - re: this item, i've not looked at the diff yet but if there's a change to the formatting output (even if it's an improvement) then we'll need to edition gate it behind style edition 2027

e.g. https://github.com/rust-lang/rustfmt/blob/777e25a8c91af9923ad356eb4448ac2ed15167a9/src/items.rs#L147

camsteffen commented 5 days ago

There is a minor change in behavior for when multiple methods are chained together - the width used in the error will be the last successfully modified width. I think this is okay?

@camsteffen - re: this item, i've not looked at the diff yet but if there's a change to the formatting output (even if it's an improvement) then we'll need to edition gate it behind style edition 2027

There should be no change in user-facing behavior, formatting. Just a change to the contents of the error object, which is currently never used IIUC.