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

Consider omitting trailing commas in function calls of one argument #3796

Closed
wchargin opened this issue Sep 14, 2019 · 6 comments
Closed

Consider omitting trailing commas in function calls of one argument #3796

wchargin opened this issue Sep 14, 2019 · 6 comments

Comments

@wchargin
Copy link

Builder patterns are common in Rust’s standard library and ecosystem.
Here are a few real (adapted) examples, with current rustfmt output
(version 1.3.0-stable):

fn main() {
    let new_commit_oid = try_invoke_git(
        Command::new("git")
            .args(&["commit-tree", &tree, "-p", "HEAD"])
            .stdin(Stdio::piped())
            .stdout(Stdio::piped())
            .env("GIT_CONFIG_NOSYSTEM", "1"),
    );
    let app = clap::App::new("my_app").arg(
        Arg::with_name("verbose")
            .long("verbose")
            .help("Print more stuff"),
    );
    let data = parse_event_files(
        WalkDir::new(logdir)
            .follow_links(true)
            .same_file_system(true)
            .max_depth(10),
    );
}

Unfortunately, in all these cases the trailing comma is actively
unhelpful. The common modification pattern for such builders is not
adding an additional argument to the enclosing function call, but rather
additional mutations to the builder: like .stderr(Stdio::piped()) to
Command, or .short("v") to Arg, or .contents_first(true) to
WalkDir. In such a case, one must remove the trailing comma
inserted by rustfmt before adding the desired change, and of course
rustfmt will insert a new trailing comma after that.

I’m a big advocate for trailing commas when they make sense, which is
most of the time—but the friction here is notable. Perhaps rustfmt
could accommodate this pattern by omitting trailing commas in function
calls of one argument. Python’s black is quite similar to rustfmt in
both spirit and output, and it follows this guideline (as of v19.3);
here’s an example.

I will be happy to contribute this change if it would be entertained.

(#3166 is related, but this is a feature request for a general pattern.)

@topecongiro
Copy link
Contributor

Thank you for creating a feature request to rustfmt! I am sorry to say, however, that we are unlikely to accept this feature. One of the core design principles of rustfmt is consistency, and I feel that this feature does not have enough benefit to break it.

one must remove the trailing comma
inserted by rustfmt before adding the desired change

Concerning a builder pattern, I think that we can just add a new element in the middle of the method chain, instead of appending it. That way the trailing comma should not bother you.

On a side note, it seems to me that the current behavior of black is just bugged, see psf/black#826 or psf/black#493.

@wchargin
Copy link
Author

Okay, understandable. Thank you for your quick reply! Adding calls to
the middle of the chain of course isn’t ideal, but maybe it’s all right.

Thanks also for the references to Black’s behavior; that’s great to
know! :-)

@sweihub
Copy link

sweihub commented Nov 26, 2022

Hi, I am looking for the solution, any way to disable the trailing comma inside function call? It bothers me much.

fn on_rsp_user_login(
    &mut self,
    pRspUserLogin: *mut CThostFtdcRspUserLoginField,
    pRspInfo: *mut CThostFtdcRspInfoField,
    nRequestID: ::std::os::raw::c_int,
    bIsLast: bool,     <==== HERE, I DON'T WHAT THIS COMMA!
) {
    debug!("user login");
    self.tx.send(Event::UserLogin).unwrap();
}

@ytmimi
Copy link
Contributor

ytmimi commented Nov 26, 2022

@sweihub you can try setting trailing_comma=Never, however this will remove all trailing commas.

fn on_rsp_user_login(
    &mut self,
    pRspUserLogin: *mut CThostFtdcRspUserLoginField,
    pRspInfo: *mut CThostFtdcRspInfoField,
    nRequestID: ::std::os::raw::c_int,
    bIsLast: bool
) {
    debug!("user login");
    self.tx.send(Event::UserLogin).unwrap();
}

@sweihub
Copy link

sweihub commented Nov 29, 2022

@ytmimi Hi, thanks for your suggestion, unfortunately, it dose't work at all, and I'm using the nightly build already.

The trailing_comma in document
https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#trailing_comma

trailing_comma is a misleading name, it says How to handle trailing commas for lists

@ytmimi
Copy link
Contributor

ytmimi commented Nov 29, 2022

@sweihub trailing_comma=Never should do what you're looking for. run rustfmt --config=trailing_comma=Never path/to/file.rs. or add trailing_comma = "Never" to your project's rustfmt.toml.

The examples listed in the docs use functions. Perhaps the docs need some clarification, but How to handle trailing commas for lists refferes to any general list of items. In this case, the list of function parameters.

If you'd like to continue the discussion I think opening a new issue would be best.

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

No branches or pull requests

4 participants