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

Should formatting be customisable? How? #3

Closed
nrc opened this issue Aug 23, 2016 · 32 comments
Closed

Should formatting be customisable? How? #3

nrc opened this issue Aug 23, 2016 · 32 comments

Comments

@nrc
Copy link
Member

nrc commented Aug 23, 2016

In the short-term, we need ways to experiment with how Rustfmt formats. However, what should the long term look like? There are wins if all code looks the same in terms of adjusting to new projects, and this approach is taken by Gofmt. On the other hand, we can never please everyone with a single formatting style, and people using Rustfmt with different styles is better than people not using Rustfmt at all (and the Rust community is not as 'conformant' as the Go community); this approach is taken by ClangFormat.

Currently, we have a rustfmt.toml file with many disparate options. If we do decide to keep some customisation, we should at the least decide on a strategy for what kind of options we want to present and how they are organised.

Finally, we have some options for introducing 'speed bumps' to customisation, we could:

  • only allow customisation on nightly (vs stable or beta), this somewhat follows stabilisation policy for Rust)
  • only allow customisation on Rustfmt build from source, rather than from distributed binaries
  • only allow customisation of rustfmt, but not cargo fmt (I think this one is a terrible idea, but brain-storming...)
  • opt-in to customisation with a command line argument or environment variable
  • rustfmt distributed with Rust (some day, assuming this happens) does not permit customisation, but you can download a separate version which does.
  • sure there are others

The idea with these speed bumps is we more strongly encourage users to use the default formatting, but permit customisation where it is necessary (e.g., corporate pressure, interaction with existing codebases, bloody-mindedness)

@joshtriplett
Copy link
Member

This also ties in with the RFC procedure. If RFC proposers have to add an option to rustfmt for the new behavior, then the new behavior must necessarily have an associated option.

For something we don't want to support customization of, and want to require a single style for, we don't want an option. Given that, we'd either need to use one of the procedures @nrc suggested above to avoid exposing that option (e.g. only allowing it in nightly), or allow the RFC proposer to instead provide a rustfmt PR that changes the behavior to implement the proposal without adding an option.

@steveklabnik
Copy link
Member

I am in favor of no customization options, personally. That said, I do recognize that this is a harder fight since we've waited so long.

@wycats has said it before, but basically, if a significant amount of the community wants to customize rustfmt, we've failed. And the standard library certainly should use it without options, to lead by example.

@joshtriplett
Copy link
Member

@steveklabnik I agree entirely. Formatting is such a bikeshed topic that I think we're better off making a reasonable call (one that at least a significant subset of people prefer) and declaring it as "the style". Consistency has more value than the vast majority of individual formatting decisions.

@bruno-medeiros
Copy link

@steveklabnik @joshtriplett I disagree, I think rustfmt should offer some degree of customization. Yes, we should have a default/preferred Rust style, but I think rustfmt should allow some deviation from that preferred style.

One good example is max-line-width. This is more than a bikeshed option, there is no way one can determine a default value that works well for everyone, even if people are forced to "get used to it". This is because preferred max-line-width will be heavily dependent on the users monitor and IDE/editor setup. Some users might work a lot from a laptop, on the go, as such not have much monitor width available because of smaller monitor, and thus prefer a small max-line-width. Other teams might work in fancy offices with huge dual widescreen monitors, and heaploads of monitor width available. Another variation on this is users liking tinier fonts or not. Or using fullscreen editors, vs editors/IDEs with additional side-views.

Another case that was mentioned in different issue and will most likely require an option as well, is whether rustfmt should break string literals - or more generally, if the formatting can change the token structure of the source code. I think by default it should only change whitespace, and only with an extra option should it break string literals and other similar tokens.

There are likely to be more niche formatting aspects where no customization options might be too restrictive in practice.

@steveklabnik
Copy link
Member

steveklabnik commented Sep 5, 2016 via email

@bruno-medeiros
Copy link

@steveklabnik And because Ruby does it, does that necessarily mean it's a good thing for Rust to do it as well? I'm skeptical of that. Unless you explicitly want to enable a similar culture and mentality as the Ruby community has - which I would argue is bad thing. Rust and Ruby seem one of the most opposite languages in the language spectrum I can think of. (I should probably come clean in that I am very biased against Ruby and dynamic languages in general - but that is a subject preference, yes, can't say much more about it)

One objective observation though, is that Ruby is in general much, much less verbose than Rust.
As such, one can argue that setting 80 as max cols is probably fine for the vast majority of Rubyist because it is a relatively large max line width for Ruby, no?

@steveklabnik
Copy link
Member

I'm saying that I have previous experience in other language communities
where this has worked well, not that Rust should always do what Ruby does.
There's a large number of reasons to choose a single, global style, even if
it's not absolutely perfect in specific situations.

You are right that Rust is much more verbose than Ruby; that's why the
original 4/100 setting was chosen. That said, a lot of people do write 2/80
or 4/80 code in Rust. I'm not sure that the need for 100 is actually real.

On Tue, Sep 6, 2016 at 10:34 AM, Bruno Medeiros notifications@github.com
wrote:

@steveklabnik https://github.com/steveklabnik And because Ruby does it,
does that necessarily mean it's a good thing for Rust to do it as well? I'm
skeptical of that. Unless you explicitly want to enable a similar culture
and mentality as the Ruby community has - which I would argue is bad thing.
Rust and Ruby seem one of the most opposite languages in the language
spectrum I can think of. (I should probably come clean in that I am very
biased against Ruby and dynamic languages in general - but that is a
subject preference, yes, can't say much more about it)

One objective observation though, is that Ruby is in general much, much
less verbose
than Rust.
As such, one can argue that setting 80 as max cols is probably fine for
the vast majority of Rubyist because it is a relatively large max line
width for Ruby, no?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABsilY4o29gP1PM_eDINxUaHzvXQAXuks5qnXncgaJpZM4JrVwq
.

@nrc nrc added the P-high label Sep 15, 2016
@gnzlbg
Copy link

gnzlbg commented Sep 15, 2016

I use some unconventional formatting: 2 spaces/80 char hard line limit. From past discussions I think that 4 spaces / 100 char hard line limit is going to win, which obviously hurts my feelings a bit.

Still, even if a style that I hate gets approved, I am strongly against customization options of any kind.

Consistency, and not having to have a style discussion ever again makes us all more productive. This is more important than my feelings.

In those particular cases in which one wants to manually format something, a escape hatch in form of e.g. a scoped comment (// rust-fmt off/on) is enough.

@bruno-medeiros
Copy link

bruno-medeiros commented Sep 15, 2016

Still, even if a style that I hate gets approved, I am strongly against customization options of any kind.

Consistency, and not having to have a style discussion ever again makes us all more productive. This is more important than my feelings.

Enforcing consistency on language code on a global level seems like "totalitarianism" to me, so to speak. Consistency should be enforced on a project, team or organizational level, but not globally. (and ideally this would similar or identical to the standard/global style - but we don't live on a ideal world)

@gnzlbg
Copy link

gnzlbg commented Sep 15, 2016

Enforcing consistency on language code on a global level seems like "totalitarianism" to me

It is, style just becomes part of the language, which isn't necessarily bad.

Consistency should be enforced on a project, team or organizational level, but not globally.

Why? What do we gain by not enforcing it globally? What do we loose if we enforce it globally?

The moment your project/team/organization uses an external crate with a different style one needs to adapt to a new style, from reading source code, to debugging, fixing bugs, enhancing the crate, and then there is the issue of maintaining an internal fork and merging upstream changes (either you end up with multiple styles within your organization or you need to reformat back and forth).

I just think that the benefits of a single style greatly outweight the cost of "getting used to it during the first week".

@Hrothen
Copy link

Hrothen commented Sep 15, 2016

Enforcing a style globally doesn't actually mean it gets used globally, it just means people who don't like it can't use the standard formatting tool.

There are basically no benefits to having a single global style, it's not actually hard to adapt to different styles in different projects. With formatter config files you don't even need to think about it.

@gnzlbg
Copy link

gnzlbg commented Sep 15, 2016

it just means people who don't like it can't use the standard formatting tool.

Just because you don't like the style doesn't mean you cannot use it.

I am not going to like the official style, and probably nobody will, because not a single person is going to decide the style, but it will be arrived at by consensus.

Still, even if users were able to customize the style, experience with clang-format (which is highly customizable) shows that they are still not going to like the result because the tool won't be magic (for example, it won't be able to infer that some macro declares a matrix or a tensor and needs to be formatted differently), it won't have all the options they want, and it will have bugs that will screw up formatting in some places.

So if "What if I don't like it" was a problem, allowing users to customize the style would not solve it. But I don't think it is a problem, I think it is a reality, since it has no solution.

As said above, I am not going to like the style that it is agreed upon, but I will still use it, and I am sure that after a week of using it I won't think about it again. Every programmer I know has been able to adapt to a different style when moving to a different project in a matter of days.

I am also sure that those who won't use the standard formatting tool will be a very tiny minority because the benefits of an automatic formatting tool is just too large.

There are basically no benefits to having a single global style, it's not actually hard to adapt

But then you still have to adapt. The benefit is no needing to adapt, ever, nor having to think about style ever again.

@Hrothen
Copy link

Hrothen commented Sep 15, 2016

Just because you don't like the style doesn't mean you cannot use it.

Yeah, but it does mean that I won't use it for any of my own stuff.

I am also sure that those who won't use the standard formatting tool will be a very tiny minority because the benefits of an automatic formatting tool is just too large.

Many many people do not use formatting tools at all for any language, they are the vast majority.

@gnzlbg
Copy link

gnzlbg commented Sep 15, 2016

Yeah, but it does mean that I won't use it for any of my own stuff.

Of course, on that I agree, but chances are you will still use it.

For example, Haskell's hindent started supporting multiple styles, but this feature, originally implemented because its main author did not like the most widely used Haskell style (Johan Tibell's), has been removed in version 5 (blogpost, might be relevant to this discussion). Now it supports one single style only.

Many many people do not use formatting tools at all for any language, they are the vast majority.

This heavily depends on the language (e.g., see Go or C#), but I disagree, everybody inputs code as text at some point, and most programs for doing this have some level of code formatting support.

@Hrothen
Copy link

Hrothen commented Sep 15, 2016

For example, Haskell's hindent started supporting multiple styles, but this feature, originally implemented because its main author did not like the most widely used Haskell style (Johan Tibell's), has been removed in version 5 (blogpost with the arguments, might be relevant to this discussion). Now it supports one single style only.

Yeah, I was part of that fight. hindent isn't actually used by the majority of haskell programmers, the change started a huge fight, and the project was forked. Also Tibell's style isn't the most widely used, many people just have personal styles that are fairly similar to it.

@gnzlbg
Copy link

gnzlbg commented Sep 15, 2016

Yeah, I was part of that fight.

For anybody interested, here the reddit dicussion. Lot's of arguments are given there both in favor and against an universal formatting style for Haskell (just keep in mind that rust is not haskell).

Also Tibell's style isn't the most widely used, many people just have personal styles that are fairly similar to it.

Sorry about that, I (and most projects I look at) use a very similar style, but I learned that it was called "Tibell" the first time I read that blog post.

and the project was forked

Is the fork maintained/developed? (It looks dead).

@perlun
Copy link

perlun commented Sep 19, 2016

For reference, this ("should we allow configuration or not") has already been debated at length here: rust-lang/rfcs#1607. I really don't think we should repeat all of that discussion once again. 😄 The arguments for both sides have been very well explained there, so please at least read all of that thread before writing more about it.

@aturon summed it up like this (my emphasis on the most relevant part in his comment):

The core team is ready to move this RFC into final comment period 💬

By and large, there appears to be agreement on the mechanics proposed here. The main point of contention has been a question at the core of this effort: do we view the outcome of this process as a strongly-recommended set of defaults? The exact way that we "strongly recommend" the defaults is an important but ultimately secondary issue. To come to any consensus about the process, we need to know what kind of status we intend to give it.

In particular, if this process does inform a community-wide recommendation, then everyone who has strong stylistic preferences has incentive to participate; we should all want to arrive at a solution where we're not tempted to customize the rustfmt defaults for our own projects.

From my read of the excellent comment thread so far, it seems that most participants do think the outcome of this process should be a recommended set of defaults. However, most participants also think that communicating this recommendation by not providing knobs by default is a step too far.

During FCP, let's try to draw the above discussion to a conclusion (in particular, speak up if you disagree strongly with the above!) And of course, if you see other major concerns, now is the time to raise them.

So, in my eyes, this has already been settled - we should provide a set of recommended defaults, but we should also make it possible to override these defaults if/when needed. (without the so-called "speed bumps")

@joshtriplett
Copy link
Member

@perlun Thanks for the reference! I agree that we shouldn't retread that territory. So, it sounds like our approach should be to have consensus defaults, strongly recommend but don't mandate those defaults, and offer options for things people strongly want to customize.

@nrc
Copy link
Member Author

nrc commented Sep 19, 2016

So, in my eyes, this has already been settled

As I stated on the RFC, I don't think that was an appropriate place to have that discussion, and don't feel it was finished. In particular, unless you were involved with that discussion already, it was very easy to miss that it covered the defaults question.

In any case, I feel like the important question is not whether we have options (at the least, we must do so to some extent in order to experiment with questions in this process, and there are likely to be some options we want to allow people to customise), but what is the appropriate speed bump to change away from the defaults, and whether it is worth deciding up front on which options we should offer (or guidelines for deciding which options to offer).

@nrc
Copy link
Member Author

nrc commented Sep 19, 2016

We discussed this at the style team meeting today. Consensus was that we should allow customisation and that we should not impose any kind of speed bump to doing so. The hope is that cultural pressure will encourage people to stick to the default style.

I propose that RFC PRs should include which alternatives should be supported by Rustfmt and how (i.e., which options/combinations of options). The intention is that the set of options should be smaller and more coherent than at the moment.

@gnzlbg
Copy link

gnzlbg commented Sep 20, 2016

Consensus was that we should allow customisation and that we should not impose any kind of speed bump to doing so. The hope is that cultural pressure will encourage people to stick to the default style.

+1 for consensus.

I propose that RFC PRs should include which alternatives should be supported by Rustfmt and how (i.e., which options/combinations of options). The intention is that the set of options should be smaller and more coherent than at the moment.

What do you mean by "combination of options"? Something like "narrow" / "wide" styles with 80 chars / line + 2 spaces and 100 chars/line + 4 spaces?

@nrc
Copy link
Member Author

nrc commented Sep 20, 2016

What do you mean by "combination of options"? Something like "narrow" / "wide" styles with 80 chars / line + 2 spaces and 100 chars/line + 4 spaces?

Yeah, pretty much. So for that RFC, some sets of options might be, indent-width: usize, max-width: usize, indent-width: usize, max-width: usize, preferred-width: usize, comment-width: usize, or page-style: Wide | Narrow. It would be up to the RFC to specify the set of options and which are valid (for example, for option two, it might state that max-width must be >= preferred-width).

@aturon
Copy link
Member

aturon commented Sep 27, 2016

I propose that RFC PRs should include which alternatives should be supported by Rustfmt and how

Hm, I'm unsure about this -- it seems to cut against the main thrust of the team/RFCs, which is to establish a single official Rust style.

In general, I do think it makes sense to consolidate rustfmt configuration options if the tool is to remain configurable, but that seems largely to be a rustfmt implementation concern, rather than something that should be debated as part of the official style process.

@aturon
Copy link
Member

aturon commented Sep 27, 2016

Also, a couple procedural points:

  • It's helpful, when going into FCP, to leave a comment announcing as much. People don't get notified by the tag itself, and it's easy to miss (I missed it initially)
  • I'm also a little confused about what FCP on issues mean here -- is this issue basically acting as a quasi-RFC?

@nrc
Copy link
Member Author

nrc commented Sep 28, 2016

@aturon

that seems largely to be a rustfmt implementation concern

I mostly agree with this. I do want to remove as many options as possible and I feel bad about removing options without a place to discuss first.

Also, a couple procedural points:

We're basically figuring out as we go along, but my thoughts are that FCP for an issue means we think there is enough consensus to write an RFC PR. Since there will be a PR, it seems a good idea not to advertise issue FCP too widely, since it could get confusing to advertise two FCPs for the same thing. However, commenting along with the FCP tag seems like a good idea to do.

@aturon
Copy link
Member

aturon commented Sep 28, 2016

but my thoughts are that FCP for an issue means we think there is enough consensus to write an RFC PR.

Oh! I misunderstood and thought the issue was being fully settled here. If this is just consensus to start the RFC process, advertisement etc is much less important.

@wycats
Copy link

wycats commented Oct 18, 2016

We discussed this at the style team meeting today. Consensus was that we should allow customisation and that we should not impose any kind of speed bump to doing so.

The last time I participated in this discussion, I felt convinced that we didn't need a separate-package speedbump, and that simply not auto-generating the rustfmt.toml for all users would be enough. Did the style team come to consensus on auto-generating a rustfmt.toml from cargo new, or just not requiring a separate package to get the customization?

@nrc
Copy link
Member Author

nrc commented Oct 18, 2016

@wycats we didn't explicitly discuss generating a rustfmt.toml. I personally don't think we should do that - it seems like encouraging customisation rather than removing a speed bump, and it ties together Cargo and Rustfmt in a way that makes me a little bit uncomfortabl.

@keeperofdakeys
Copy link

Having a default rustfmt.toml also means it's harder to change the behaviour of rustfmt, and harder to update the configuration file format. Another (unrelated) question is whether rustfmt should encode its defaults using a rustfmt.toml internally.

@solson
Copy link
Member

solson commented Oct 18, 2016

EDIT: I realized what @wycats really meant about cargo new.

It would be cool if rustfmt could generate a rustfmt.toml in that style where it contains all the defaults (commented out) with explanations about what the various options do.

It would also probably be appreciated by some if cargo new could copy in their customized rustfmt.toml in each new project they make, although if we went that far I would say "why not give me a default README.md and my LICENSE files from a template", since that would be even more useful to me.

As for generating a rustfmt.toml by default, I think it's a bad idea. A rustfmt.toml should only be necessary for overriding default options, so it would signal to me, "there is some non-standard formatting in this repo".

@aturon
Copy link
Member

aturon commented Oct 18, 2016

@solson

It would be cool if rustfmt could generate a rustfmt.toml in that style where it contains all the defaults (commented out) with explanations about what the various options do.

This step seems potentially dangerous -- I think we're walking a thin line between actively discouraging customization (though an explicit speedbump) and actively encouraging it (by sweetening the ergonomics).

As for generating a rustfmt.toml by default, I think it's a bad idea. A rustfmt.toml should only be necessary for overriding default options, so it would signal to me, "there is some non-standard formatting in this repo".

I agree with this for sure.

But note that there's some dissonance here, where different parts of the toolchain treat customization as first-class, or not.

More broadly, I want to re-emphasize a point I made on the original RFC, which is that the existence of the style team only makes sense if we push for extremely widespread adoption of the defaults. To put it starkly, I would consider it a failure to produce a set of defaults that are commonly tweaked -- if that's the case, there's little reason to go through a formal RFC process/strike team to set these defaults.

There are a couple implications of this thinking:

  • We should be maximally conservative, at the outset, about how we treat customization. You can always smooth out speedbumps or add workflows later, but it's nigh impossible to go the other direction. Thus, I personally would advocate for starting with a speedbump of some form.
  • We should think very carefully about what kinds of customizations to allow in rustfmt in general, and how those influence the style RFC process. In particular, if we're tempted to say that a given bit of styling is an OK choice "since you can always customize", I think we've lost.

nrc added a commit that referenced this issue Oct 25, 2016
@nrc nrc mentioned this issue Oct 25, 2016
@nrc nrc added has-PR and removed ready-for-PR labels Oct 25, 2016
@ssokolow
Copy link

ssokolow commented Dec 16, 2016

I can actually present myself as an example of someone for whom "not enough customization" would result in abandoning rustfmt rather than changing style.

While I did actually adjust my style in a few places when trying out rustfmt, there were also a few situations where I either trial-and-error'd the poorly-documented rustfmt.toml options until things looked right or simply fired up git gui, committed only what I approved of, and reverted the rest.

For example, I absolutely refuse to have { on its own line unless the block exists purely to constrain the scope of a binding and, if it weren't for these options...

ideal_width = 100
fn_call_width = 100
fn_brace_style = "PreferSameLine"
where_density = "Compressed"

...I'd have either discarded rustfmt's changes in git gui or modified the function in question by factoring out where 'a: 'b since it was at fault for putting { on its own line and the function used to work acceptably when using the same lifetime for both cases. (The switch to where 'a: 'b was a "just in case"/"build a habit" best practice recommended in #rust)

In fact, because of these and certain disagreements that currently have no rustfmt options, I plan to have "format canaries" in my codebases that can help the CI to fail PRs on stylistic grounds. (eg. to catch people who enable reorder_imported_names and end up putting the all-caps integer constant imports at the beginning rather than the end.)

(I don't mind people formatting to their preference, editing, and then formatting back... so the format canaries are mainly going to be things that either can't be reliably reversed, like comment-style normalization, or don't currently have rustfmt options for the style I insist on, like sorted imports with all-caps names coming after everything else.)

I'd really love to be able to run rustfmt on every commit or even every time I save a file... but I'm not willing to compromise on certain aspects of my code style.

For example, one of the changes I'm willing to be convinced otherwise about, but currently filter out at the git gui stage is an attempt to split this across three lines:

if self::access::probably_writable(abs_path) { return Ok(()); }

(I haven't yet figured out a more idiomatic way to write it in context and keeping it on one line without regressing other single-line functions would require a smarter fn_single_line that treats return Ok(()) and INVALID_FILENAME_CHARS.chars().any(|x| x == *c) differently.)

nrc added a commit that referenced this issue Dec 20, 2016
@nrc nrc closed this as completed in #33 Jan 24, 2017
@U007D U007D mentioned this issue Mar 23, 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