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

Inconsistent formatting of method calls #1797

Closed
djc opened this issue Jul 19, 2017 · 4 comments
Closed

Inconsistent formatting of method calls #1797

djc opened this issue Jul 19, 2017 · 4 comments

Comments

@djc
Copy link
Contributor

djc commented Jul 19, 2017

The way these get formatted doesn't make sense to me:

diff --git a/src/main.rs b/src/main.rs
index 7824cad..0254a95 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -46,19 +46,21 @@
 fn check_folder(ctx: Context, label: Label) -> Box<ContextFuture> {
     let Context { client, conn } = ctx;
-    Box::new(client.call(CommandBuilder::select(&label.name))
+    Box::new(
+        client
+            .call(CommandBuilder::select(&label.name))
     )
 }

@@ -70,20 +72,25 @@
 fn sync_folders(ctx: Context) -> Box<Future<Item = Context, Error = io::Error>> {
     let Context { client, conn } = ctx;
-    Box::new(conn.prepare("SELECT id, name, mod_seq FROM labels")
+    Box::new(
+        conn.prepare("SELECT id, name, mod_seq FROM labels")
+    )
 }

Why does the former get an extra newline, and the latter does not? To me, the latter looks more readable -- wasting an entire line on an atomic expression doesn't make that much sense to me.

@nrc
Copy link
Member

nrc commented Jul 19, 2017

The reason we change this at all is that there is a width heuristic for the arguments of a function call (we're currently discussing at rust-lang/style-team#47). I'm not sure why the two cases get formatted differently, I agree they probably should both be formatted like the latter. But I think the optimal formatting is probably staying on one line.

@topecongiro
Copy link
Contributor

@djc I could not reproduce this on the latest rustfmt-nightly. Could you please try with it?

@djc
Copy link
Contributor Author

djc commented Jul 20, 2017

@topecongiro I still see it with rustfmt-nightly v0.1.9:

diff --git a/src/main.rs b/src/main.rs
index 0254a95..2fed8de 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -47,12 +47,14 @@ type ContextFuture = Future<Item = Context, Error = std::io::Error>;
 fn check_folder(ctx: Context, label: Label) -> Box<ContextFuture> {
     let Context { client, conn } = ctx;
     Box::new(
-        client.call(CommandBuilder::select(&label.name))
+        client
+            .call(CommandBuilder::select(&label.name))
             .collect()
             .and_then(|(_, client)| {
                 let cmd = CommandBuilder::fetch()

You can test with https://github.com/djc/mailsync if you want. djc/mailsync@be792c2 applied most of rustfmt's suggestions, with the exception of this case; rustfmt-nightly-0.1.9 then still wants to make this change.

@topecongiro
Copy link
Contributor

@djc Thank you for more information!
So rustfmt splits long chain and puts each element on its own line. However, if the first element is small (<= tab width), rustfmt puts the second element next to the first one.

// Do this
conn.bar()
    .baz();
// Instead of this
conn
    .bar()
    .baz();

When the chain is shorter than chain_one_line_max and is not multi-lined, rustfmt will keep it on a single line.

@nrc nrc closed this as completed Nov 2, 2017
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

3 participants