shame
shame copied to clipboard
RayMarch
fmt
This pr just rustfmt's the code. There were a few lines that rustfmt was having trouble with, I fixed those manually.
This is in preparation for another pr. Merging this pr first will make the diff of the next pr easier to read.
edit: I've rebased onto current main and made a couple changes based on your concerns. Changes are outlined in the commit messages below. I believe I've seen to all instances of case 4 using the method I mentioned below. I've not seen to cases 0-3. I don't think there are solutions for those.
ok i spent some hours trying to make rustfmt less aggressive on this codebase, and i made some good progress.
right now i'm using cargo +nightly fmt
current rustfmt.toml i'm using is this:
unstable_features = true
force_multiline_blocks = false
format_macro_bodies = false
fn_single_line = true
max_width = 120
There are some outstanding formatting issues that i can't seem to get working: (there might be more later as soon as we resolved those)
case 0: blown up for loops
for_range_step(0..10, || 1, |i: int| {
a += i;
});
becomes
for_range_step(
0..10,
|| 1,
|i: int| {
a += i;
},
);
i'm trying to keep the edsl concise, but this is blowing things up and makes it difficult to read imo.
case 1: comments getting moved around
move || { //<--
a += 1.0;
},
becomes
move || {
//<--
a += 1.0;
},
arrow points nowhere, it was supposed to point out the move keyword line. Maybe a /*<--*/ style comment would help here, but i'd prefer if this could be fixed in the formatter.
case 2: things getting forced into one line just because they fit in the line limit
pub fn rec_error(err: shame_graph::Error) {
shame_graph::Context::with(|ctx| {
ctx.push_error(err)
});
}
becomes
pub fn rec_error(err: shame_graph::Error) { shame_graph::Context::with(|ctx| ctx.push_error(err)); }
I'm not sure how to make rustfmt less aggressive and maybe make it accept that the original code was already within the line limit, and that this may have been intentional. There are other cases where long lines like this might be appropriate so i don't want to turn the limit down. Not sure how to deal with the "either everything is this long or nothing is this long" behavior. maybe it can be turned off somehow?
case 3: one line trait impls
impl ScalarOrSame for (vec2, scal) {type Widest = vec2;}
impl ScalarOrSame for (vec3, scal) {type Widest = vec3;}
impl ScalarOrSame for (vec4, scal) {type Widest = vec4;}
impl ScalarOrSame for (mat2 , scal) {type Widest = mat2;}
impl ScalarOrSame for (mat2x3, scal) {type Widest = mat2x3;}
impl ScalarOrSame for (mat3x2, scal) {type Widest = mat3x2;}
impl ScalarOrSame for (mat2x4, scal) {type Widest = mat2x4;}
[...more impls]
becomes
impl ScalarOrSame for (vec2, scal) {
type Widest = vec2;
}
impl ScalarOrSame for (vec3, scal) {
type Widest = vec3;
}
impl ScalarOrSame for (vec4, scal) {
type Widest = vec4;
}
impl ScalarOrSame for (mat2, scal) {
type Widest = mat2;
}
[...more impls]
this one makes spotting typos in the type names quite tricky to me. I'm not sure i want to introduce macros just because of formatting...
case 4: macro bodies (SOLVED => skip_macro_invocations)
impl_lhs_rust_primitive_type_mul!(
Ten<_, f32> * f32,
Ten<_, i32> * i32,
Ten<_, u32> * u32,
);
becomes
impl_lhs_rust_primitive_type_mul!(
Ten < _,
f32 > *f32,
Ten < _,
i32 > *i32,
Ten < _,
u32 > *u32,
);
i wish there was a setting to stop it from formatting macro calls altogether, i don't want to have to add #[rustfmt::skip] every time i use a macro. Theres no use in it trying to interpret whats in the macro since its made up syntax anyways. I couldn't find the setting. hope there is one.
I'd be glad if you could help me out with these remaininig problems. I was quite surprised how much i could already fix with these few settings, but the remaining issues are kind of a blocker for me. I'm open for discussion about this topic though.
My sincere advice is to give up control. Use the defaults. Sorry, I probably should have said this before you spent the time and frustration trying to bend rustfmt you your will.
case 0
The majority of your users will be using default settings. This shape is what your users code will look like. This is ok. I know you want it to look like a for loop but it's not a for loop, its a function call. Don't worry, it looks fine.
case 1
Rustfmt and rustdoc see comments as annotating either a specific expression, or no expression. According to idoms, that comment is annotating a += 1.0;. This is ok.
case 2
This behavior is intentional. This property is usually upheld:
for all a, b:
(ast(a) = ast(b)) <-> (fmt(a) = fmt(b))
case 3
This is a semi-arbitrary stylistic choice that rustfmt makes so we don't have to. I suggest just going with it.
case 4
Use curly braces.
impl_lhs_rust_primitive_type_mul! {
Ten<_, f32> * f32,
Ten<_, i32> * i32,
Ten<_, u32> * u32,
}
x!(); x![]; and x!{} are all valid ways to invoke a macro. From what I've seen these seem to be the idioms: () hints to readers that it's like a function call. [] is for list-ish things (often used for serde_json::json! too). {} is for other stuff.
The fact that this wasn't obvious is a bug in rustfmt. Somebody should submit a pr to mention this behavior in the docs for skip_macro_invocations.
oh don't get me wrong, i am quite certain i will give up the control. But it can't hurt to explore the space of options a bit before i let the tool override all my intentional formatting. Since the tool is destructive theres no simple way of going back to restore things.
I'm sorry this isn't happening faster.