rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

[unstable option] brace_style

Open scampi opened this issue 6 years ago • 27 comments
trafficstars

Tracking issue for unstable option: brace_style

scampi avatar Feb 13 '19 22:02 scampi

Seems brace_style = "AlwaysNextLine" does not work on unsafe blocks.

misos1 avatar Dec 13 '19 15:12 misos1

Seems brace_style = "AlwaysNextLine" does not work on logic blocks either.

Sibz avatar Sep 09 '20 08:09 Sibz

@Sibz could you elaborate on what you mean by logic blocks, with a code snippet? If you're referring to things like if, else etc. then you'll want to check out the control_brace_style option https://rust-lang.github.io/rustfmt/?version=master&search=#control_brace_style

calebcartwright avatar Sep 15 '20 01:09 calebcartwright

Thank you, didn't see that one and I did scour the whole list! Mustn't have had my morning coffee.

Sibz avatar Sep 15 '20 20:09 Sibz

New structs aren't included in this i.e.

let value = MyStruct {
    something: 1
}

vs

let value = MyStruct
{
    something: 1
}

Sibz avatar Sep 30 '20 09:09 Sibz

Closures are also not included in this.

foo.ok_or_else(|| {
  // closure
})

Should be:

foo.ok_or_else(|| 
{
  // closure
})

Iron-E avatar Aug 04 '21 20:08 Iron-E

Sorry for double comment. Also does not work for macro_rules!:

macro_rules! filter_map_view {
  ($query:ident, $val:ident) => {

Should be:

macro_rules! filter_map_view 
{
  ($query:ident, $val:ident) => 
  {

Iron-E avatar Aug 05 '21 00:08 Iron-E

Also doesn't work for use.

Input

Using brace_style = "AlwaysNextLine":

use ::
{
	std::time::Duration,
	async_std::
	{
		fs::remove_file,
		path::Path,
		task,
	},
	clap::
	{
		app_from_crate,
		crate_authors,
		crate_description,
		crate_name,
		crate_version,
		Arg,
	},
	log::
	{
		error,
		info,
	},
};

Expected

No changes.

Actual Result

use ::{
	async_std::{
		fs::remove_file,
		path::Path,
		task,
	},
	clap::{
		app_from_crate,
		crate_authors,
		crate_description,
		crate_name,
		crate_version,
		Arg,
	},
	log::{
		error,
		info,
	},
	std::time::Duration,
};

benaryorg avatar Aug 23 '21 13:08 benaryorg

At this point I think it's worth pausing to reconsider whether the current option is the right approach to meet the needs of users. For reference, this option was historically only applied to items; not in the expression nor statement contexts. control_brace_style also exists to configure the brace style on control flow type expressions.

We'd started extending the option to cover a few additional expression contexts, such as on unsafe, async, and extern blocks. However, I'm increasingly getting the sense that having only one option (essentially) is unlikely to provide a sufficient amount of flexibility. I wonder if instead we might need to a tiered approach that provide more options with more granular control, as well as a top level "big hammer" option that cascades everywhere unless overridden by a more granular one (similar to the width heuristic options). For example, while some folks may want every brace to always be on its own line everywhere, others may indeed just want the style to apply to items and not say the bodies of their multiline match arms.

This option as originally intended and implemented, controlling brace style for items, is actually probably a fair candidate for stabilization at this point, but if we need to extend it out to cover every expression context as well then it's likely to remain unstable for years to come.

calebcartwright avatar Aug 26 '21 02:08 calebcartwright

I think some of the confusion comes from the qualified naming of control_brace_style and unqualified naming of brace_style. You might consider renaming this option to item_brace_style if that is the intention, and move unsafe, async, and extern support to a new brace_style.

Iron-E avatar Aug 28 '21 20:08 Iron-E

I wonder if instead we might need to a tiered approach that provide more options with more granular control, as well as a top level "big hammer" option that cascades everywhere unless overridden by a more granular one

That sounds great.

This option as originally intended and implemented, controlling brace style for items, is actually probably a fair candidate for stabilization at this point

I hope this does not mean that it cannot be renamed.

misos1 avatar Sep 05 '21 20:09 misos1

I hope this does not mean that it cannot be renamed.

Guess I'd missed this, but no, it has no impact. Even stabilized options can be renamed, provided we can do so without running into name conflicts with other options, as we can do such renames in a non-breaking manner.

Obviously unstable options can be renamed or even removed as necessary.

calebcartwright avatar May 13 '22 00:05 calebcartwright

It would also be good to have a bit more specificity, for instance, be able to apply it only to certain types of elements and ignore others, for example:

Good

pub struct Elevator { // Would be nice to have a way to ignore it applying to structs 
    projector: Projector,
    height_map: GrayImage,
}

impl Elevator
{
    pub fn new(radius: f64, origin: Coordinate, height_map: GrayImage) -> Self
    {
        Self {
            height_map,
            projector: Projector::new(radius, origin),
        }
    }
}

Bad

pub struct Elevator
{
    projector: Projector,
    height_map: GrayImage,
}

impl Elevator
{
    pub fn new(radius: f64, origin: Coordinate, height_map: GrayImage) -> Self
    {
        Self {
            height_map,
            projector: Projector::new(radius, origin),
        }
    }
}

milewski avatar Sep 29 '23 04:09 milewski

Is there a reason why RUST put's it's open bracket on the same line. Is it a performance thing? It's definitely not easier to read. Most languages place it on a new line, why change what works? I prefer the old method, unless someone knows why we shouldn't.

SnipeHub avatar Feb 04 '24 05:02 SnipeHub

Is there a reason why RUST put's it's open bracket on the same line. Is it a performance thing? It's definitely not easier to read. Most languages place it on a new line, why change what works? I prefer the old method, unless someone knows why we shouldn't.

It is just a matter of personal taste. Also I prefer it on a new line. Just seems the author of rustfmt probably prefers it differently. 4 years passed and this is still not resolved, still no progress? Other tools like for example linters even for javascript have options, but not rustfmt, this is what holds me back from using this otherwise seemingly great tool (for 4 years now).

misos1 avatar Feb 04 '24 06:02 misos1

Is there a reason why RUST put's it's open bracket on the same line. Is it a performance thing? It's definitely not easier to read. Most languages place it on a new line, why change what works? I prefer the old method, unless someone knows why we shouldn't.

It is just a matter of personal taste. Also I prefer it on a new line. Just seems the author of rustfmt probably prefers it differently. 4 years passed and this is still not resolved, still no progress? Other tools like for example linters even for javascript have options, but not rustfmt, this is what holds me back from using this otherwise seemingly great tool (for 4 years now).

It appears it's probably not a very important feature for them if it's remained unstable for 4 years. I guess I'll just have to press the space bar one extra time, not a big deal. I'll pass on this until they decide to fix it.

SnipeHub avatar Feb 04 '24 06:02 SnipeHub

Hey all 👋🏼, just wanted to chime into the discussion with a little bit of info.

Is there a reason why RUST put's it's open bracket on the same line. Is it a performance thing? It's definitely not easier to read. Most languages place it on a new line, why change what works? I prefer the old method, unless someone knows why we shouldn't.

The default choice isn't based on any one persons opinion. rustfmt is primarily an implementation of the Rust Style Guide and the default formatting adheres to the guide. A lot of discussion has gone into it, and it's a living document that continues to evolve. These days, the guide is owned by the rust style team (check out the link for more details on the team).

ytmimi avatar Feb 08 '24 18:02 ytmimi

The default choice isn't based on any one persons opinion. rustfmt is primarily an implementation of the Rust Style Guide and the default formatting adheres to the guide. A lot of discussion has gone into it, and it's a living document that continues to evolve. These days, the guide is owned by the rust style team (check out the link for more details on the team).

Ok so it is the opinion of some group of people. I think one of reasons is for crates to have consistent (the same) formatting and that is probably a good point. But this point should not apply to people using crates programming their projects, they can have their own style guides for their projects.

misos1 avatar Feb 08 '24 18:02 misos1

Ok so it is the opinion of some group of people

Yes and no. Style decisions are made after coming to a consensus. Anyone is free to open PRs to discuss new default styles. And just to be clear, the style guide only prescribes the default style. I was simply trying to answer the earlier question and clarify that the rustfmt team isn't making style related decisions.

ytmimi avatar Feb 08 '24 18:02 ytmimi

Is there a reason why RUST put's it's open bracket on the same line. Is it a performance thing? It's definitely not easier to read. Most languages place it on a new line, why change what works? I prefer the old method, unless someone knows why we shouldn't.

It is just a matter of personal taste. Also I prefer it on a new line. Just seems the author of rustfmt probably prefers it differently. 4 years passed and this is still not resolved, still no progress? Other tools like for example linters even for javascript have options, but not rustfmt, this is what holds me back from using this otherwise seemingly great tool (for 4 years now).

It appears it's probably not a very important feature for them if it's remained unstable for 4 years. I guess I'll just have to press the space bar one extra time, not a big deal. I'll pass on this until they decide to fix it.

I'm in the same situation I guess. This is one of the most basic formatting features, and it's still in unstable despite it already works well. Currently I made a workaround in my projects, a small shell script that switches to nightly, formats code, then switches backs to stable toolchain.

How can I contribute to speed this up? I'm not familiar with rustfmt code at all, but I have some middle-level rust experience. What parts are currently missing to have full approval in stable?

MassiminoilTrace avatar Apr 11 '24 10:04 MassiminoilTrace

Currently I made a workaround in my projects, a small shell script that switches to nightly, formats code, then switches backs to stable toolchain.

I have no idea what you're talking about. Running a nightly rustfmt is as easy as cargo +nightly fmt, provided that you have installed a nightly toolchain and enabled unstable formatting features. It also doesn't affect the rest of your code in any way, you can easily stay on stable Rust while using unstable formatting options.

The only issue with unstable options is that they are not guaranteed to achieve Rust's quality standard, and not guaranteed to never change formatting between toolchain releases. Neither are likely to be an issue for this specific feature.

afetisov avatar Apr 17 '24 00:04 afetisov

brace_style = PreferSameLine || SameLineWhere control_brace_style = AlwaysSameLine do not work if the preceding element is multi-line (e.g. common when chain_width = 0)

Note 1: the .method in the preceding code means that the start of the branch code will not be confused with the multi-line element above it. Note 2: I have Visual indenting on, but this is true regardless.

Is this intended? And if so may I suggest it not be or request a workaround?

Examples:

Current

Current_1

Desired

Desired_1

Current

Current_0

Desired

Desired_0

dream-dasher avatar Jun 01 '24 02:06 dream-dasher