Fix/Extra whitespace in nested pub extern #5529
Conversation
…to the flow of the rewrite_assign_rhs_expr function.
…s_with function (expr.rs - 1977) if the lhs expression ends with whitespace and the rhs expressions starts with whitespace.
|
@ytmimi Noticed that |
|
Seems like a networking issue. No worries, I just reran that job. |
| if context.config.version() == Version::Two && lhs.ends_with(" ") && rhs.starts_with(" ") { | ||
| return Some(lhs + &rhs.trim_start()); | ||
| } |
There was a problem hiding this comment.
Although this technically fixes the issue, it doesn't resolve the underlying problem. I believe the trailing space is properly added to the end of "pub extern ", but we shouldn't be adding a space before "C". I think the actual issue is occurring in choose_rhs. @calebcartwright Do you think this is the right approach?
There was a problem hiding this comment.
Okok. This approach also handles other similar cases. However, it there's a better approach I'm up to take it. I'll wait for confirmation before making any changes to choose_rhs 👍.
tests/source/issue-5525.rs
Outdated
| second_long_argument_name: u32, | ||
| third_long_argument_name: u32, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
If we're just testing that the input is idempotent then we don't need the source/ test file.
There was a problem hiding this comment.
True, mb. I'll fix this now.
There was a problem hiding this comment.
Thanks for taking this on. Right now I'd suggest holding off on making any changes to the PR until @calebcartwright has had a chance to review. the choose_rhs code path is fairly hot so we probably want to be cautious about the code changes we make there to address this issue.
…for idempotence tests.
Solves #5525
rhsexpression on therewrite_assign_rhs_withfunction (expr.rs - 1977) if thelhsexpression ends with whitespace and therhsexpressions starts with whitespace. NOTE: The change only applies if theversion: Twoconfig is provided.