rust-lang / rustfmt

Format Rust code
https://rust-lang.github.io/rustfmt/
Apache License 2.0
6.05k stars 888 forks source link

Add option to put a space after function name and generic parameters #3564

Closed RaidoWolf closed 4 years ago

RaidoWolf commented 5 years ago

For a library that I'm developing, part of the styleguide that I developed was to leave a single space after the function name and generic parameters. To my knowledge, it's not a common format, but in my opinion is more readable (especially when no syntax highlighting is available).

A couple of examples of this format are as follows:

impl Story {

    pub fn swap_context <T: 'static + Context + Send + Sync> (&mut self, context: T) -> Option<Box<Context + Send + Sync>> {
        // ...
    }

    pub fn render (&mut self, delta: f64) -> bool {
        // ...
    }

}

Forgive me if this isn't something that is appropriate for rustfmt, or if this is already possible. I looked through the documentation for the config options and didn't see anything that would affect this. I appreciate your time and any help you can offer.

EDIT: Similar to the function declarations, I'm also using similar formatting for match expressions on arms that match enums:

match self.contexts.pop() {
    Some (mut context) => {
        context.on_pop();
        return Some(context);
    }
    None => return None,
}
scampi commented 5 years ago

That style feels very specific, I don't think rustfmt should try providing an option for it. See https://github.com/rust-lang/rustfmt/issues/1974 where many old config option about spacing got merged/removed.

RaidoWolf commented 5 years ago

That's understandable, it may well be too specific to put in as a built-in option, but even if it isn't inherently supported, it should certainly be possible in some way. Is there a way to extend the functionality via external code? It would absolutely be acceptable if I had to write a plugin or something to get this functionality.

scampi commented 5 years ago

I don't think rustfmt provides a plugin interface, but that sounds like something worth having!

RaidoWolf commented 5 years ago

I've decided that this is not an important styling issue, and I've stopped using this style in my code and found that it's infinitely more beneficial to have this tool in my workflow than whatever I perceived as a minor readability improvement. It also seems like this tool has the goal of being simple and lightweight, rather than being infinitely configurable. I'm in agreement with that goal, and it's been successful for other tools such as prettier (although this is still much more configurable than that). For that reason, I'm going to go ahead and close this issue for now. Feel free to re-open it if I'm jumping the gun. Thanks for your time!

bugeats commented 4 years ago

rustfmt already has a long list of configuration parameters, and the criteria is not clear for what is important enough to include. I think rustfmt should go all-out tyranny like Prettier, or allow for plugable configuration like eslint.

Personally, I find a space after a function name to be important, and it's the only peeve I have with the rustfmt defaults.

Quick arguments for space-after-function-name:

You wouldn't write fn foo()->void would you? would you?

RaidoWolf commented 4 years ago

@bugeats I agree, especially with the point about visually distinguishing between a function definition and invocation, which was the reason that I took up that style in the first place. The reason that I closed this issue was because I'm pretty new to rust, even newer to rustfmt, and it seemed like I didn't make a great case for why it's useful, but I think your comment put that in more eloquent terms, so I'm going to re-open this. Thanks for taking the time to reply.

calebcartwright commented 4 years ago

@bugeats

and it's the only peeve I have with the rustfmt defaults.

The official style guide for rust can be found at https://github.com/rust-dev-tools/fmt-rfcs and is defined and managaed via an RFC process. rustfmt's default formatting must align to that style guide.

Some pertinent sections:

rustfmt already has a long list of configuration parameters, and the criteria is not clear for what is important enough to include.

That's a fair observation.

As mentioned above the default formatting emitted by rustfmt with zero configuration is based on the official style guide. I'd describe the approach as defining a style where the majority of the formatting prescriptions can be accepted the majority of rust developers to hopefully provide consistent formatting across as much rust code as possible.

However, config options are also provided for areas where a "sizeable" group of developers want something other than the default, with the canonical examples being spaces vs. tabs or block vs. visual indentation style.

We don't want to end up with a ton of configuration options to enable an individual preference for a rare style that's otherwise unused by the community. However, if there's a good case to be made for an alternative to the default, and/or a lot of folks wanting an alternative style then config option requests are certainly considered.

You wouldn't write fn foo()->void would you? would you?

No. function signatures are formatted according to the style guide.


Part of the challenge I see with a new config option to support the requested formatting in the OP:

I'm not personally convinced of the benefits of the requested style, but I suppose I wouldn't be vehemently opposed to a config option (with a default that still aligned to the style guide) if someone else was willing to implement it. However, I'd be surprised if any of us from the maintainers team ever did.

cc @topecongiro

dylni commented 4 years ago

@calebcartwright I've also been waiting for this option to be added.

Strong wording against it in the style guide ("do not" instead of softer wording like "prefer" and "avoid" used elsewhere in the guide)

This is true, but it could be changed if the option is added.

I've never seen the requested style in rust code before, so IMO this feels more like an uncommon individual preference vs. a style that many rustfmt users would opt into via a config option

This might be difficult to judge, since rustfmt is likely the most used tool for styling Rust code. I would use this option if it was supported, and it's always been my main issue with rustfmt.

For example, look at the voting for this issue, which shows that it's a very common preference.

extra spacing in function signatures is going to result in more signatures being unable to fit on one line

This is true, but I don't think it's a big concern. The same could be said about adding a space before {.

Also, I would find the following more readable and consistent, which would remove one of the spaces:

pub fn swap_context<T: Context> (&mut self, context: T) -> Option<Box<Context>> {
    //             ^ no space here
    // ...
}
calebcartwright commented 4 years ago

Thanks @dylni! For those folks interested in these potential config options I'd encourage indicating so via the reactions feature on the OP. I say options plural as there's already variance here between snippets posted in comments which would require at least two different config options to support.


@dylni



This might be difficult to judge, since rustfmt is likely the most used tool for styling Rust code.

I understand the point, but I don't think the chicken or the egg argument is really applicable. rustfmt is relatively new compared to rust itself, and the style guide and rustfmt's config options were largely driven off what was already common practice in rust code and considered idiomatic; not the other way way around.

For example, look at the voting for this issue, which shows that it's a very common preference.

Thanks for sharing. However, our focus is on rust developers writing rust code. Neither the style guide nor rustfmt are really going to make decisions based on other programming languages nor what formatting styles may or may not be popular with developers writing in other programming languages.

Like most things in life there's tradeoffs with configuration options too, one of which is that each option adds complexity, both for rustfmt users and within the rustfmt codebase itself. A rustfmt config option that enabled a formatting style wildly popular in programming language X, but that was never really used within rustfmt and rust, wouldn't really be pulling it's weight in this ecosystem.


Code style is inherently subjective, and although I understand some of you would prefer the extra spacing, IMHO the proposed spacing styles would still likely end up being pretty rare in practice with rust code.

All that being said, we're still open to considering this feature request. It's a pretty low priority, and has been labeled accordingly, but if anyone's interested in attempting to implement it we'd be happy to take a look!

dylni commented 4 years ago

IMHO the proposed spacing styles would still likely end up being pretty rare in practice with rust code.

This might be true, but the option adds very little code. #4302 implements the option, except for spaces before generics.

RaidoWolf commented 4 years ago

I'm closing now, as this has been implemented in MR #4302. Thanks again for everybody's help with this!