rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

[unstable option] struct_field_align_threshold

Open scampi opened this issue 6 years ago • 7 comments

Tracking issue for unstable option: struct_field_align_threshold

scampi avatar Feb 13 '19 22:02 scampi

Two issues I've noticed with this option:

First, it interacts poorly with hard_tabs. The + lines ought to be tabbed equal to len and then space-aligned afterward, but they're being given block indentation instead. Users with different tab lengths will see weird formatting.

This might be easier to visualize by copying the following examples to an editor with space highlighting.

Bad:

	.push_sized(&fuse_kernel::fuse_out_header {
		len:    (size_of::<fuse_kernel::fuse_out_header>()
			+ fuse_kernel::FUSE_COMPAT_ENTRY_OUT_SIZE
			+ size_of::<fuse_kernel::fuse_open_out>()) as u32,
		error:  0,
		unique: REQUEST_ID,
	})

Would be good:

	.push_sized(&fuse_kernel::fuse_out_header {
		len:    (size_of::<fuse_kernel::fuse_out_header>()
		         + fuse_kernel::FUSE_COMPAT_ENTRY_OUT_SIZE
		         + size_of::<fuse_kernel::fuse_open_out>()) as u32,
		error:  0,
		unique: REQUEST_ID,
	})

Secondly, I don't think block-formatted literals should be aligned -- or it should maybe be disable-able. This is how gofmt does it, and it is easier to read when working with mixed primitives and compound literals.

Example of poor formatting:

  .push_sized(&fuse_kernel::fuse_entry_out {
    // ...
    entry_valid_nsec: 0,
    attr_valid_nsec:  0,
    attr:             fuse_kernel::fuse_attr {
      ino: 11,
      ..Default::default()
    },
  })

What I would expect:

  .push_sized(&fuse_kernel::fuse_entry_out {
    // ...
    entry_valid_nsec: 0,
    attr_valid_nsec:  0,
    attr: fuse_kernel::fuse_attr {
      ino: 11,
      ..Default::default()
    },
  })

jmillikin avatar Jul 31 '20 11:07 jmillikin

Is there a similar option for function parameters?

SOF3 avatar Sep 12 '22 16:09 SOF3

Will this ever be made available in stable Rust?

wusticality avatar Mar 30 '23 05:03 wusticality

@wusticality please see https://github.com/rust-lang/rustfmt/discussions/5365

ytmimi avatar Mar 30 '23 15:03 ytmimi

Thanks for adding this option. An issue I have noticed whilst using it is that the formatting isn't applied when using ..Default::default(). For example, I get:

StandardMaterial {
    base_color_texture: Some(asset_server.load("textures/card_back.png")),
    cull_mode: Some(Face::Front),
    fog_enabled: false,
    unlit: true,
    alpha_mode: AlphaMode::Blend,
    ..Default::default()
}

whereas I would expect:

StandardMaterial {
    base_color_texture: Some(asset_server.load("textures/card_back.png")),
    cull_mode:          Some(Face::Front),
    fog_enabled:        false,
    unlit:              true,
    alpha_mode:         AlphaMode::Blend,
    ..Default::default()
}

theon avatar Feb 18 '24 21:02 theon

@theon thanks for sharing. When you have a chance can you create a new issue outlining the problem in more detail. I think it would be best to track this in a separate thread.

ytmimi avatar Feb 18 '24 21:02 ytmimi

Thanks @ytmimi, I have raised https://github.com/rust-lang/rustfmt/issues/6080

theon avatar Feb 18 '24 23:02 theon