Fix overlong function signature #1089
Conversation
|
Hi, consider following test case: impl Handle {
pub fn merge(a: Abcd) -> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::Edge> {
}
}I suspect, that there is more than one-by-off issue. We can be under |
|
You are right. I try to fix it tomorrow. |
| @@ -1359,8 +1359,9 @@ fn rewrite_fn_base(context: &RewriteContext, | |||
| _ => { | |||
| // If we've already gone multi-line, or the return type would push | |||
| // over the max width, then put the return type on a new line. | |||
| result.contains("\n") || multi_line_ret_str || | |||
| result.len() + indent.width() + ret_str_len > context.config.max_width | |||
| let overlong_ret = result.len() + indent.width() + ret_str_len > | |||
There was a problem hiding this comment.
Could you add a comment explaining why we need the -1
|
Will continue to work on this and the other issue next weekend. No spare time until then. |
|
Nightly seems to have problems but i think it is not because of my changes. Edit: master does not compile on nightly either. |
|
Yeah, the nightly issue is probably a bug in Rust. |
|
And in fact that should be fixed now. I restarted the tests here. |
|
|
||
| // If there is no where clause, take into account the space after the return type | ||
| // and the brace. | ||
| if where_clause.predicates.is_empty() { |
There was a problem hiding this comment.
Do we need to check if the return type exists? I.e., not do this if there is no return type?
Also, nit, brace = {}, I think you mean parenthesis in this comment and above.
There was a problem hiding this comment.
Yes, you are right. No return type means no arrow means no additional space.
There was a problem hiding this comment.
Isn't this checked on ln 1354 already? if !ret_str.is_empty()
There was a problem hiding this comment.
Oh, I overlooked that... 😅 but to be sure I will add a test for that case later.
|
@dawirstejeck whats left to do here? lgtm |
|
@juicejitsu you are probably right. But thank you for having a look at it! PS: I'm sorry for being so slow. I love to participate but I only got 1 or 2 hours per week 🐌. |
|
@juicejitsu, @dawirstejeck: does this PR cover everything that #1108 does? Is there a reason to land one or the other? |
|
@nrc yeah this covers the bug in that PR as well, although there are some extra test case that are not in this one. |
|
Thank you! |
Fix off-by-one error in rewrite_fn_base. #1049
Also remove a line break.