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

Left floating chain call #566

Closed
matklad opened this issue Nov 6, 2015 · 16 comments
Closed

Left floating chain call #566

matklad opened this issue Nov 6, 2015 · 16 comments

Comments

@matklad
Copy link
Member

matklad commented Nov 6, 2015

For my code, rustfmt applies this formatting:

let program = glium::Program::from_source(&display,
                                          &include_str!("./shaders/vertex.glsl"),
                                          &include_str!("./shaders/fragment.glsl"),
                                          None)
                  .unwrap();

This unwrap feels a little lonely :(

I don't know a way of formatting this really well, but this two options are somewhat better in my opinion

let program = glium::Program::from_source(
    &display,
    &include_str!("./shaders/vertex.glsl"),
    &include_str!("./shaders/fragment.glsl"),
    None
).unwrap();
let program = glium::Program::from_source(&display,
                                          &include_str!("./shaders/vertex.glsl"),
                                          &include_str!("./shaders/fragment.glsl"),
                                          None).unwrap();
@nrc
Copy link
Member

nrc commented Nov 6, 2015

I agree that 1 is sub-optimal. 2 would require a different option (I can't recall which one off the top of my head). I think we could do 3, I wonder how generally applicable it would be? The downside is that the unwrap gets kind of lost amongst the arguments.

@leoyvens
Copy link

let program = glium::Program::from_source(&display,
                                          &include_str!("./shaders/vertex.glsl"),
                                          &include_str!("./shaders/fragment.glsl"),
                                          None)
                             .unwrap();

This might not be the prettiest solution but it's more general.

@LaylBongers
Copy link

Running across this issue myself (interestingly with the exact same function), I can't actually find the option to format it like 2 mentioned by @nrc. I went over the options multiple times trying different ones to see if they would work but none did.

@marcusklaas
Copy link
Contributor

There's no such option yet, at this moment. Personally, I'm not super keen to implement them at this point as they have their own downsides. I'd rather further explore our options here.

@LaylBongers
Copy link

Currently I'm now using the following in my own code:

let program = Program::from_source(
        &display,
        include_str!("vert.glsl"),
        include_str!("frag.glsl"),
        None
    )
    .unwrap();

This allows chaining as much behind it as I would want and doesn't look weirdly detached. It does however conflict with what overall is the default in rustfmt, I'm just mentioning it as an additional option.

@matklad
Copy link
Member Author

matklad commented Jan 2, 2016

I think that the core of a problem here is that aligning function arguments to ( is not a scalable solution.

Consider

let mut g_buffer = MultiOutputFrameBuffer::with_depth_buffer(api.facade,
                                                             [("color", &g_albedo)]
                                                                 .iter()
                                                                 .cloned(),
                                                             &g_depth);

this looks awful even without unwrap.

How such code is formatted in the wild? I found this example.

@crazymykl
Copy link

Speaking for myself, 2 is the most readable of the three.

@matklad
Copy link
Member Author

matklad commented Mar 14, 2016

There are some more opinions (in favor of 2) here: https://users.rust-lang.org/t/how-do-you-format-multy-argument-function-calls/4946/2

@nrc
Copy link
Member

nrc commented Apr 21, 2016

While this proposal still has the same problem as we currently have, it is definitely an improvement, I wonder if it is worth doing in the short term?

@nrc
Copy link
Member

nrc commented May 13, 2016

@crumblingstatue are you hitting this with the new release (0.5) or 0.4?

@crumblingstatue
Copy link
Contributor

crumblingstatue commented May 13, 2016

@nrc c1949f5, which should be the same as 0.5, minus the version bump

@nrc
Copy link
Member

nrc commented May 13, 2016

Thanks!

@retep998
Copy link
Member

The lack of option 2 with rustfmt is one of the big blockers preventing me from using rustfmt.

@topecongiro
Copy link
Contributor

rustfmt now offers option 2 with fn_call_style = "Block".

@topecongiro
Copy link
Contributor

Closing since with the defualt RFC style we do not have floating chain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants