rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

Per-type skip

Open nilgoyette opened this issue 6 years ago • 6 comments

As discussed in #2924 and "hinted" in #2555 and maybe others, a per-type skip would be nice. I don't have tens of use-case, in fact I have only one: the matrix. In our computer graphic application, we define a lot of matrices in several modules, thus making #[rustfmt::skip] uninteresting. What I (and probably many others) want is something like

// in lib.rs or mod.rs. Or top of a file?
#[rustfmt::skip(Matrix3, Matrix4)]

because I can't imagine a single case where I would want rustfmt to format my matrices. The only right way to format a 3x3 matrix is when it has 3 columns and 3 rows. All in one line or in one row is unreadable.

I don't know rustfmt code at all, nor what it could possibly do. Do you have access to the base type when parsing the files? I ask because I wonder if an alias type Affine = Matrix3<f64> would force the user too add all aliases in the skip, if such a feature existed.

nilgoyette avatar Feb 01 '19 19:02 nilgoyette

It could be also another config option akin to ignore where the struct's name would be specified.

I don't think the type is available so the ignore wouldn't propagate to Affine in your example. @topecongiro is that the case ?

scampi avatar Feb 02 '19 11:02 scampi

Since rustfmt acts on AST, which holds little or no type information, skipping based on type can only be done heuristically.

if an alias type Affine = Matrix3<f64> would force the user too add all aliases in the skip

This won't happen unless the user explicitly uses #[rustfmt::skip(Affine)]. Attributes inside external crates won't have effect.

topecongiro avatar Feb 10 '19 15:02 topecongiro

In general I think columns of numerical data are an area where code formatters struggle. There is more semantic information when these are formatted correctly and it makes bugs and incorrect literals stand out, which is what good syntax should do. I can see the argument that having these special snowflake formattings messes up an otherwise orderly plan for formatting, but this is a case where the standard formatting really does make the code much harder to use. This might be a niche edge-case for Rust in general, but for people who want it, I'd argue it really matters.

Also it would be fantastic if Rust could actually generate clean matrix formatting for us :)

brian-armstrong avatar Jul 16 '20 10:07 brian-armstrong

What about something like #[rustfmt::arrayNxM] where rustfmt will automatically arrange things matrix style, with configuration for horizontal alignment of the columns in .rustfmt.toml? As a first pass this could be only for [T; {N*M}] but could be expanded for argument lists to functions and tuples/tuple structs.

This is also useful for vertex buffers or raw image data where data comes in pairs, triples, etc, but may be stored as just [T; {len * M}]

glfmn avatar Feb 17 '22 18:02 glfmn

I think this is a more general issue. There are a lot of places where readability improves with column alignment,

Take this example:

let expanded = ConfigKeys {
    program_id: keys[0],
    group_id: keys[1],
    cache_id: keys[2],
    user_id: keys[3],
    user_meta: keys[4],
    user_ev: keys[5],
    user_queue: keys[6],
}

or

let expanded = ConfigKeys {
    program_id : keys[0],
    group_id   : keys[1],
    cache_id   : keys[2],
    user_id    : keys[3],
    user_meta  : keys[4],
    user_ev    : keys[5],
    user_queue : keys[6],
})

blueforesticarus avatar Jul 30 '22 12:07 blueforesticarus

I'm not sure the decision of when to columnate is something that can be chosen automatically. But perhaps a directive like #[rustfmt(align)] could exist to specify blocks that should be formatted this way. similar to the #[rustfmt::arrayNxM] suggestion @glfmn made

blueforesticarus avatar Jul 30 '22 12:07 blueforesticarus

Hey @nilgoyette, I don't know if it works in your use case but I've seen something like the following:

#[rustfmt::skip]
mod matrices {
    pub const ARR1: [ /* hand formatting */ ];
    pub const ARR2: [ /* hand formatting */ ];
}

use matrices::*;

It's a bit less elegant since you lose an indent and all items have to be contiguous, but at least you don't need to call out each individual array.

tgross35 avatar Feb 02 '23 08:02 tgross35