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

Nesting of block indented expressions #1572

Merged
merged 4 commits into from
May 23, 2017
Merged

Conversation

topecongiro
Copy link
Contributor

@topecongiro topecongiro commented May 21, 2017

Closes #1422, closes #1449, and closes #1494.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

This looks great, thanks for tackling it! I wonder if tuples need covering and if there are any other constructs that need covering. Other than that, this looks good to merge

src/expr.rs Outdated
}
}
_ => &**body,
ast::ExprKind::Call(_, ref args) => (args.len() == 1, &**body),
ast::ExprKind::Closure(..) => (true, &**body),
Copy link
Member

Choose a reason for hiding this comment

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

Should we handle tuples here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tuples are not refered in rust-lang/style-team#61, but if we should, do we use "a single expression parameter" rule or allow exception like clousres?

// A single expression tuple
let a = foo(bar((|x| {
    let y = x + 1;
    let z = y + 1;
    z
})));

// Should we allow this?
let a = foo(bar((b, c, |x| {
    let y = x + 1;
    let z = y + 1;
    z
})));

Copy link
Member

Choose a reason for hiding this comment

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

I think we should allow both (note that tuples with a single element must always have a trailing comma).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to make it clear, should we use block indent for tuples when fn_call_style = Block and format like function call? (c.f. rust-lang/style-team#64)

let a = (
    xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx,
    yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy,
);

Copy link
Member

Choose a reason for hiding this comment

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

yes, I think so

@topecongiro
Copy link
Contributor Author

I have added tuple and block. Also added a commit to add a trailing comma to a single arg in multiline.
Now rewriting tuple uses the same function as rewriting function call when fn_call_style = Block.

@nrc
Copy link
Member

nrc commented May 23, 2017

Awesome, thank you!

@nrc nrc merged commit 7c8432f into rust-lang:master May 23, 2017
@topecongiro topecongiro deleted the nested-block branch May 23, 2018 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants