Fix formatter reordering comments in function type parameters #5229
Conversation
| r#"pub fn main() -> #( | ||
| String, | ||
| // This is a string | ||
| String, |
There was a problem hiding this comment.
Test fix as per comment in conversation here: #5225 (comment)
|
Hey @robertDurst! Could you add a changelog entry (see existing entries for inspiration)? |
robertDurst
changed the title
Done! Thanks for catching this @ankddev ... had the |
robertDurst
force-pushed
the
rdurst/5225
branch
from
43bb33f to
65fbd1b
Compare
|
Ok, cleaned up the commit history, should be GTG for an initial review. Thanks in advance and happy holidays 🥳 🎄 |
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Thank you!! I've left a small comment inline
compiler-core/src/format.rs
Outdated
| } | ||
| } | ||
|
|
||
| fn wrap_arguments_with_line_breaks<'a, I>( |
There was a problem hiding this comment.
How come this is needed when none of the other matching syntaxes need this? Can we not use the same approach for all of them?
There was a problem hiding this comment.
Thanks for the review! Let me take another pass here - had a nice few days off here on holiday, refreshed and with a clear mind 🧘
robertDurst
force-pushed
the
rdurst/5225
branch
from
65fbd1b to
4926043
Compare
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Thank you! I've left a note inline about the approach.
| .append(")") | ||
| } else { | ||
| self.wrap_arguments(arguments, location.end) | ||
| } |
There was a problem hiding this comment.
It looks like this duplicates the code within wrap_arguments. Why is it that that code didn't do the job?
I think we should work out what the bug was there so it works for all (...), rather than having a special case where the code is replicated here.
3 participants
Address #5225 .
Specifically, this PR:
comment_in_tuple_return_typeper conversations in Formatter reoders comments inside function types #5225.Tried my best to follow of convention. Long term goal here is to hopefully continue finding places to contribute, so nit picks of all sizes appreciated 😁