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

RFC process for formatting style and Rustfmt defaults #1607

Merged
merged 1 commit into from Aug 15, 2016

Conversation

Projects
None yet
@nrc
Copy link
Member

nrc commented May 4, 2016

This RFC proposes a process for deciding detailed guidelines for code
formatting, and default settings for Rustfmt. The outcome of the process should
be an approved formatting style defined by a style guide and enforced by
Rustfmt.

Rendered

This RFC proposes a process for deciding detailed guidelines for code
formatting, and default settings for Rustfmt. The outcome of the process should
be an approved formatting style defined by a style guide and enforced by
Rustfmt.

This RFC proposes creating a new repository under the [rust-lang](https://github.com/rust-lang)
organisation called fmt-rfcs. It will be operated in a similar manner to the
[RFCs repository](https://github.com/rust-lang/rfcs), but restricted to
formatting issues. A new [sub-team](https://github.com/rust-lang/rfcs/blob/master/text/1068-rust-governance.md#subteams)
will be created to deal with those RFCs. Both the team and repository are
expected to be temporary. Once the style guide is complete, the team can be
disbanded and the repository frozen.

@nrc nrc added the T-core label May 4, 2016

@NotaseCretagen

This comment has been minimized.

This process is specifically limited to formatting style guidelines which can be
enforced by Rustfmt with its current architecture. Guidelines that cannot be
enforced by Rustfmt without a large amount of work are out of scope, even if
they only pertain to formatting.

This comment has been minimized.

@Manishearth

Manishearth May 4, 2016

Member

Is this necessary? I'm okay (might want to check with other maintainers) with having clippy constrained by the style rfcs, and allowing any kind of style rfc that can be enforced by rustfmt or clippy (which together should cover most style guidelines except for those pertaining to overall design).

I do intend to write up a huge clippy rfc for deciding defaults at some point, but if this existed it might be possible in a more incremental fashion.

This comment has been minimized.

@nrc

nrc May 4, 2016

Author Member

I considered a broader focus for the style team - including lints and unenforceable conventions. However, after some discussion, we thought it would be better to keep the style team as focussed as possible in order to get a quick resolution and to be really clear about the scope of the process.

for Rust community members. However, it risks overwhelming the RFC repo with
style debate.

Use issues on Rustfmt. I feel that the discussions would not have enough

This comment has been minimized.

@Manishearth

Manishearth May 4, 2016

Member

IMO a little obscurity for bikeshed-prone issues is somewhat a good thing. But I can see why we'd want to not do that.

@regexident

This comment has been minimized.

Copy link
Contributor

regexident commented May 4, 2016

This answers my question on how to choose the right defaults for my take on rust-lang/rustfmt#298 then, I guess. 👍

@durka

This comment has been minimized.

Copy link
Contributor

durka commented May 4, 2016

Has anyone considered a kind of "crater for rustfmt"? We could take a sampling of crates and see how large the diff is when applying various defaults. That would give a sense of what the community actually thinks about style guidelines, when styling code by hand.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented May 6, 2016

One question about the scope of RFCs: I imagine that there will be some topics -- such as appropriate places for line-breaking -- that cut across a lot of different areas, but which ought nonetheless to be considered as a unit. It seems like this would be a good topic for an RFC, but it seems to run somewhat counter to this wording:

RFCs should be self-contained and coherent, whilst being as small as possible to keep discussion focused. For example, an RFC on 'arithmetic and logic expressions' is about the right size; 'expressions' would be too big, and 'addition' would be too small.

@aturon

This comment has been minimized.

Copy link
Member

aturon commented May 6, 2016

I'm really excited about this approach to settling on a consensus format. While rustfmt is a useful tool even if everyone configures it differently, (1) defaults matter and (2) achieving community buy-in for a standard format can reduce needless friction in the ecosystem (e.g., as experienced when moving between projects that are styled in radically different ways). Standardized formatting, of course, been a point of pride for Go; I'm excited about taking a Rustaceous approach to get there, deciding via community-driven RFCs.

The RFC doesn't emphasize it other than in the summary, but a key point is that the proposed subteam is temporary and has a very clear end goal -- the production of a style guide that is enforced by rustfmt (at least in its default configuration). This is the first proposal for a "strike team":

Over time, we may want to expand the set of subteams, and it may make sense to have temporary "strike teams" that focus on a particular, limited task.

I think this is a great model for focusing the community's attention on an area that needs specific, limited work.

I was initially skeptical about introducing a separate RFC repo, but the detailed process that @nrc is proposing seems quite plausible, and does seem to profit from separating out the traffic from our existing venues. I like that we both end up with a single coherent specification (the style guide) but also have the repo for historical information about how we arrived at that consensus.

Probably my biggest concern is making sure we have a strong "overall vision" and not just isolated RFCs that don't coordinate well -- but I see that @nikomatsakis just made essentially the same point.

@ubsan

This comment has been minimized.

Copy link
Contributor

ubsan commented May 6, 2016

As a counter to the strong overall vision; I would worry, as a user of Rust, that any team made up of core Rust people would end up making a style which is similar to rustc style (which I personally, and many rust people I know, don't like), which I don't think is the right way to go. I'd much rather be able to speak in an open, RFC style, versus being dictated to.

@aturon

This comment has been minimized.

Copy link
Member

aturon commented May 6, 2016

@ubsan I think there must be some misunderstanding -- I wasn't suggesting that the overall vision should be set outside of the RFC process -- in fact I was saying the opposite. The RFCs in the strike team shouldn't necessarily be limited in the way that this RFC suggests, but should instead include style striketeam RFCs on the larger vision that the entire community can weigh in on.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented May 6, 2016

IIRC the reason people don't like rustc style is because it isn't a style at all; it's a mishmash of styles from various points of time in the evolution of rust. New rust PRs seem to have a style more or less consistent with the ecosystem. Legacy style is irrelevant here.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented May 6, 2016

I, like @aturon, was somewhat skeptical of a whole subteam and extra repo for coordinating this, but due to what I suspect is the high level of traffic this will receive that we'll want a high signal-to-noise ratio which somewhat necessitates splitting it from the main repo. In other words, it sounds like a reasonable idea to me!

@ubsan

This comment has been minimized.

Copy link
Contributor

ubsan commented May 7, 2016

@aturon Ah, then I'm fine :) I just didn't want a core team to override the feelings of the community.

@Manishearth No, I'm talking modern rustc style.

@ticki

This comment has been minimized.

Copy link
Contributor

ticki commented May 7, 2016

There is one thing I think rustfmt does wrong: removing trailing commas.

@solson

This comment has been minimized.

Copy link
Member

solson commented May 9, 2016

@ticki The only place I can think where rustfmt removes trailing commas is after curly-brace-blocks in match arms. In other contexts, like multi-line arrays or function argument lists (at definitions or calls), it adds commas so that every line ends in a comma.

@ticki

This comment has been minimized.

Copy link
Contributor

ticki commented May 9, 2016

It removes it in match blocks, enums, and struct IIRC.

@solson

This comment has been minimized.

Copy link
Member

solson commented May 9, 2016

@ticki I don't believe you recall correctly. See http://is.gd/b9IQdf and hit the format button. It makes sure every enum variant, every struct field, and every non-block match arm has a comma.

Like I said before, it removes the trailing comma after the curly-brace-block match arm only.

@durka

This comment has been minimized.

Copy link
Contributor

durka commented May 9, 2016

It also removes trailing commas from function argument lists (declarations
and calls).

On Mon, May 9, 2016 at 3:38 PM, Scott Olson notifications@github.com
wrote:

@ticki https://github.com/ticki I don't believe you recall correctly.
See http://is.gd/b9IQdf and hit the format button. It makes sure every
enum variant, every struct field, and every non-block match arm has a
comma.

Like I said before, it removes the trailing comma after the
curly-brace-block match arm only.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1607 (comment)

@solson

This comment has been minimized.

Copy link
Member

solson commented May 9, 2016

@durka Ah, yes, because it doesn't block-indent multiline argument lists by default. Nothing with the trailing delimiter on the same line would have a trailing comma. Presumably it would keep trailing commas in the case where you enabled that option, like this:

foo(
    some_long_expression(),
    long_enough_to_be_multiline(),
);

EDIT: If it removes them in this case with this option, it's just a simple bug/inconsistency with the handling of trailing commas elsewhere. Anyway, this is definitely a detail too specific for this general RFC.

@durka

This comment has been minimized.

Copy link
Contributor

durka commented May 9, 2016

No, it still removes them in that case. Anyway this isn't important to
discuss here.

On Mon, May 9, 2016 at 4:02 PM, Scott Olson notifications@github.com
wrote:

@durka https://github.com/durka Ah, yes, because it doesn't
block-indent multiline argument lists by default. Nothing with the trailing
delimiter on the same line would have a trailing comma. Presumably it would
keep trailing commas in the case where you enabled that option, like this:

foo(
some_long_expression(),
long_enough_to_be_multiline(),
);


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1607 (comment)

* the language, tools, and libraries sub-teams (since each has a stake in code style),
* large Rust projects.

Because activity such as this hasn't been done before in the RUst community, it

This comment has been minimized.

@brson

brson Jun 8, 2016

Contributor

Typo: "RUst"

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 8, 2016

This basically looks ok to me, with a few concerns:

This establishes an official team, with a page on the website and all, for a very narrow purpose. What's the purpose of creating a team? Do we feel that people won't be motivated to work on it if they don't get a picture on the web site? Do we feel that the team won't be seen as legitimate?

This RFC focuses on breaking down style decisions into very-fined grained decisions, but style is not such a local decision - every style decision affects the appearance of every other. I'd like this RFC to address how the overall quality will be kept in check. Just as an example, perhaps the team is maintaining a rustfmt'd integration test that exhaustively shows all styles together, that gets reviewed with each new RFC, or maybe a corpus of real-world code that does the same.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jun 8, 2016

Hmm, not that I care too much, but I did not envision this strike team being listed on the web page along with the other teams, nor do I envision having the other "random bits of stuff" that come with a subteam (e.g. git alias). This is just because it's work to maintain and the team is temporary.

I think the purpose of having a team is just to clarify where the ultimate decision-making power on the matter of formatting decisions resides. (Like any team, I would expect that the process would be open, and would take into account objections from the entire community.)

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jun 8, 2016

So @wycats raised an interesting point in discussion. He -- and many others I know -- are of the opinion that rustfmt should not have any options; or, at minimum, if you want to have options, you should have to install a distinct tool (rustfmtx) to get them. The reason is basically that the value of having a common formatting tool is greatly enhanced if using the default options is basically universal. gofmt is an obvious example of a tool that has embraced this philosophy.

Now, it seems like settling this question is orthogonal to this particular RFC -- at least, it seemed so to me, at first. I imagined that we would settle this default question separately (personally, I'd probably like to remove the options, because I buy into that reasoning above, but I don't care that much tbh).

But @wycats pointed out that deciding on a particular default or format setting is very different in a context where you expect people to configure from another context. So if we wind up making this decision after a number of formatting decisions have been made, we might find we want to revisit those decisions.

Therefore, I'd like to propose that we discuss this question of whether to remove configurability a bit. I could imagine adding such a clause into this RFC itself. I could also imagine raising this question as one of the first points of discussion for the new team.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jun 8, 2016

(Other than the point about options, I think that all of @rust-lang/core at least is pretty comfortable with the idea of formatting forming a subteam.)

EDIT: Corrected amusing typo. :)

@regexident

This comment has been minimized.

Copy link
Contributor

regexident commented Jun 8, 2016

… formatting a subteam.

😂

Now, it seems like settling this question is orthogonal to this particular RFC -- at least, it seemed so to me, at first. I imagined that we would settle this default question separately (personally, I'd probably like to remove the options, because I buy into that reasoning above, but I don't care that much tbh).

Wouldn't an option-less rustfmt require us to decide on a formatting profile, too, one way or another?

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jun 27, 2016

@8573 I don't think anyone is currently arguing for never providing knobs in rustfmt (speak up if you are!). So yes, I was referring to the speedbump.

@perlun

This comment has been minimized.

Copy link

perlun commented Jun 28, 2016

@aturon - I think you summed it up quite well there. No objections from my side.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jun 29, 2016

The RFC still frames the strike team as a new subteam. I prefer not to set the precedent that 'strike teams' are subteams. I do want to empower people to make decisions, and to give them credit, but prefer not to create increasingly special-purpose teams. Can we remove the language making the strike team a subteam? For comparison, the unsafe strike team RFC does not establish a new subteam. Both of these are going to be setting precedent.

I also think it is desirable for strike team RFCs to lay out the conditions for concluding their work and disbanding, similar to charters for things like IETF work groups.

@ubsan

This comment has been minimized.

Copy link
Contributor

ubsan commented Jun 30, 2016

@brson I agree with the "strike teams are not subteams", but for a different reason. I think that the subteams should start to step down over the next ten years, and strike teams should start to take their place - specialized teams which can focus on a specific area, rather than the "lang team, responsible for designing new language features". This would bring our model more in line with modern C++, which is a language that's had a crazy transformation from C++03 to C++14 standardization processes, and whose standardization process works really well.

I don't agree strike teams should lay out the conditions for concluding their work, in other terms.

@ben0x539

This comment has been minimized.

Copy link

ben0x539 commented Jul 12, 2016

rustfmt seems pretty uninteresting to me if it's not a ticket to having my code formatted in a way that's acceptable to 100% of the rust community. there's formatters for all kinds of languages and most people totally ignore them and most lang communities never agree on configuring them, gofmt probably only works because you don't have to think about formatting politics.

future generations of rust programmers won't thank us if we fragment the community along formatting lines just to preserve the preferred style of present-day codebases

@hoodie

This comment has been minimized.

Copy link

hoodie commented Jul 12, 2016

I'm sorry if this has already come up, I've only skimmed this thread, but I'd love to see a method to disable automatic formatting in certain places, something like #[cfg(no_formatting)].
Especially repetitive test code does often not benefit from formatting example.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Jul 12, 2016

@nrc

This comment has been minimized.

Copy link
Member Author

nrc commented Jul 12, 2016

@brson it's unclear to me exactly what it means that a strike team is or is not a sub-team. I mean they are both teams of people working on a facet of Rust, they both have authority (thought not total authority in either case) to make decisions. Clearly a strike team should have limited scope in both time and jurisdiction, but that doesn't seem enough to explicitly say "is not".

As far as I see it, calling a strike team a (temporary) sub-team is a way to re-use the technical and social infrastructure we have in place, rather than having to recreate that infrastructure. If there are specific aspects of being a sub-team you think we'd be better not to emulate, then I think it makes sense to call them out, that seems preferable to starting from scratch with strike teams.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jul 13, 2016

@nrc I'm warming up to the idea of lots of teams, some of them being temporary. I wouldn't mind adding an unsafe team and rustfmt team to teams.html, as long as their scope is clear, and as a project we're making this expansion intentionally, and the other RFC is aligned. It feels though like there are policy issues to be resolved about what it means to be a team that are larger than this RFC.

I've left some similar comments on the other RFC.

@darkstalker

This comment has been minimized.

Copy link

darkstalker commented Jul 13, 2016

Whatever style you pick here, please don't use egyptian brackets. It just looks horrible everywhere.

@solson

This comment has been minimized.

Copy link
Member

solson commented Jul 13, 2016

@darkstalker This comment thread isn't the right place to work out which particular style choices should be default. Hold those thoughts for the fmt-rfcs process this RFC is proposing.

I feel compelled to add, "looks horrible" comments ought to be ignored. If you want to have a voice, at least bring something meaningful to the conversation.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jul 18, 2016

I'm 👍 on the idea of having a strike team to work out the "default and recommended style" for Rust programs.

I am fine with rustfmt permitting knobs by default, but I think we should strongly encourage people not to use them. I suspect few people will anyhow, both because it would bring their code out of sync with the mainstream, and because it's just that much more annoying to have to set them up.

I do wonder: once we settle on a default style, I imagine there may be future questions that arise -- or corner cases that were not considered. Is there a plan for resolving those questions? Maybe this won't really be a big problem?

@wuranbo

This comment has been minimized.

Copy link

wuranbo commented Jul 19, 2016

Could vertically align be considered? Many people love it.
https://shkspr.mobi/blog/2014/11/why-i-vertically-align-my-code-and-you-should-too/

Especially in the match block with one line branches.

@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Jul 19, 2016

This discussion isn't for defining the standards, just for defining the process by which we define the standards.

I would suggest opening an issue on rustfmt itself to make this an option first.

@nrc

This comment has been minimized.

Copy link
Member Author

nrc commented Jul 31, 2016

@aturon It seems that the conclusion to the 'should there be knobs' is unchanged, and a qualified 'yes'. I would suggest this should be clarified in one of the first style team RFCs. Another question that has been asked is 'what, precisely is the duration of the team and what happens after?' I suggest that the duration should be one year, and at the end of that time responsibility for style team issues should go the tools team, unless the core team decide otherwise at the time. The objective is that there should be a fairly complete set of style guidelines by then, and that these will be in maintenance mode after a year (I foresee changes being needed due to new language features and due to technical changes as Rustfmt evolves. I expect both could be managed by PRs to the style repo, shepherded by the tools team).

Are there other questions that need resolving here? If not and the above is acceptable, I can update the RFC.

@liigo

This comment has been minimized.

Copy link
Contributor

liigo commented Aug 4, 2016

Some thoughts:

  • I don't think it's urgent to focus on formatting code, for now. (Just continue doing it for experimental.)
  • Are there largely crates on crates.io have problems (bad styles) that hurt readability/cooperation? What will be solved?
  • It's a shame to enforce people formatting their code in an arbitrary style.
  • There are many natural controversies in code style, most subjective. e.g. 80-column limit or not, { at new line or not. It's hard to say which one is right, or which one is more readable.
  • The community will never get consensus about that, unless there are dictators (we hate dictators).
  • By unify code style in an authoritarian way, you solve little problems, and you get little gain. At the same time, you dissever the community. You finally get a unified style, since people who dislike the style have left the community.
  • People have rights to format their code in a style they like, unless the style is 'wrong'. (So please focus on defining which style is wrong.)
  • Some people from core team have proposed that, will not allow configuring format style by default. I worry about this. (And I claim here: I'll never use a format tool if it forbid configuration.)
  • I don't think a 'default style' or 'recommended style' should be a style which force people to use.
  • The default rustfmt tool SHOULD allow configuration.
@Manishearth

This comment has been minimized.

Copy link
Member

Manishearth commented Aug 4, 2016

Please read the final RfC decision. All the points you mention have been discussed before; the final comment period is not for rehashing resolved discussions.

It's a shame to enforce people formatting their code in an arbitrary style.

Nobody is doing that.

Some people from core team have proposed that, will not allow configuring format style by default. I worry about this.

This is not the final decision, if you read the thread. The decision is to have a "strongly recommended set of defaults". The usage of the word "defaults" implies that it is configurable.

@vitiral

This comment has been minimized.

Copy link

vitiral commented Aug 11, 2016

Can't we just hire Rob Pike to dictate the format we should use?

I think having a solidified, discussed and final format is critical for rust's growth. I strongly support the strike team and think this is something that needs to be solved as soon as possible.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Aug 15, 2016

Huzzah! The @rust-lang/core team has decided to accept this RFC. The main controversial issues have been about what kinds of knobs to provide, how to make them accessible, and the "permanency" and scope of the strike team being forced.

The general conclusions:

  • it's ok to allow projects to customize their formatting with a default rustfmt install, but we should strongly encourage the use of defaults;
  • the duration will be roughly a year and tools team will pick up after that (though I think the point is more about when some initial set of consensus is reached than a specific period of time; we can revisit if that doesn't feel right).

@nikomatsakis nikomatsakis merged commit 73b4897 into rust-lang:master Aug 15, 2016

@brson brson referenced this pull request Sep 23, 2016

Merged

Add the style team #523

@nrc

This comment has been minimized.

Copy link
Member Author

nrc commented Oct 3, 2016

For those on this thread who are interested in participating in the style/formatting process, it is now underway and the style team has been formed. The repo where we're working is https://github.com/rust-lang-nursery/fmt-rfcs and we discuss in #rust-style on irc.

@chriskrycho chriskrycho referenced this pull request Dec 31, 2016

Closed

Document all features in the reference #38643

0 of 17 tasks complete

@chriskrycho chriskrycho referenced this pull request Mar 11, 2017

Closed

Document all features #9

18 of 48 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment