Skip to content

Format const array as block if remaining width is less than half #751

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented Jan 9, 2016

Motivated by poor formatting in Manishearth/rust-clippy#540.

@@ -1591,7 +1624,8 @@ pub fn rewrite_assign_rhs<S: Into<String>>(context: &RewriteContext,
lhs: S,
ex: &ast::Expr,
width: usize,
offset: Indent)
offset: Indent,
try_block: bool)
Copy link
Member

Choose a reason for hiding this comment

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

Could you use a new enum instead of a bool here please? It makes the caller code easier to read.

@nrc
Copy link
Member

nrc commented Jan 10, 2016

Looks good and I think this is a definite improvement in the formatted output. There's a bunch of nits to fix, but they're all pretty minor. r+ with them addressed

@marcusklaas
Copy link
Contributor

I don't know if we should hold off merging this, but in my mind we could be a lot more general in our approach. Instead of adding heuristics for arrays, we could implement a more general heuristic for assignments where we break early if this ends up taking fewer lines.

For example:

looooooooooooooooooooooooooooooooong_name = funk(a,
                                                 b,
                                                 c);

would become

looooooooooooooooooooooooooooooooong_name =
    funk(a, b, c);

@nrc
Copy link
Member

nrc commented Jan 10, 2016

That is probably a good idea, we'd keep arrays formatted consistently then and handle non-array cases. I wonder if there are any cases where this PR helps, but which are not assignments?

@nrc
Copy link
Member

nrc commented Jan 28, 2016

ping @sanxiyn What do you think about @marcusklaas 's suggestion? Do you think this PR would help even if we were to implement that?

@sanxiyn
Copy link
Member Author

sanxiyn commented Feb 1, 2016

@marcusklaas's suggestion sounds good to me. I will try to implement it. There seems to be an issue(#497) for this already?

I think this PR is orthogonal, but I also think it is better implemented after line breaking at assignment is done.

@marcusklaas marcusklaas mentioned this pull request May 12, 2016
@marcusklaas
Copy link
Contributor

Superseded by #893.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants