Open wackbyte opened 5 years ago
I'm all for consistency and nicety! It's true that there are some issues such tab/space inconsistencies here and there (which I failed to see in my editor and should fix indeed).
I've not been using rustfmt nor am I planning to, for reasons I'll attempt to describe here :
I'm doing my best to follow the Rust style conventions, but sometimes there are pieces of code that should (in my opinion) break the rule, for the sake of readability. Let's get started with this small example, the implementation of flipped_z()
in bezier.rs
:
self.into_vector().map(|mut p| {p.z = -p.z; p}).into()
I wrote it as a one-liner and it should stay that way, because the operation is so simple and very quick to parse (visually). rustfmt
would probably turn this into 4 lines just because of the block in the closure.
The biggest ones are in mat.rs
:
pub fn new(
m00: T, m01: T, m02: T, m03: T,
m10: T, m11: T, m12: T, m13: T,
m20: T, m21: T, m22: T, m23: T,
m30: T, m31: T, m32: T, m33: T
) -> Self {
Self {
rows: CVec4::new(
Vec4::new(m00, m01, m02, m03),
Vec4::new(m10, m11, m12, m13),
Vec4::new(m20, m21, m22, m23),
Vec4::new(m30, m31, m32, m33)
)
}
}
...
pub fn map2<D,F,S>(self, other: Mat4<S>, mut f: F) -> Mat4<D> where F: FnMut(T, S) -> D {
let m = self.$lines;
let o = other.$lines;
Mat4 {
$lines: CVec4::new(
Vec4::new(f(m.x.x, o.x.x), f(m.x.y, o.x.y), f(m.x.z, o.x.z), f(m.x.w, o.x.w)),
Vec4::new(f(m.y.x, o.y.x), f(m.y.y, o.y.y), f(m.y.z, o.y.z), f(m.y.w, o.y.w)),
Vec4::new(f(m.z.x, o.z.x), f(m.z.y, o.z.y), f(m.z.z, o.z.z), f(m.z.w, o.z.w)),
Vec4::new(f(m.w.x, o.w.x), f(m.w.y, o.w.y), f(m.w.z, o.w.z), f(m.w.w, o.w.w))
)
}
}
...
pub fn determinant(self) -> T where T: Copy + Mul<T,Output=T> + Sub<T,Output=T> + Add<T, Output=T> {
// http://www.euclideanspace.com/maths/algebra/matrix/functions/determinant/fourD/index.htm
let CVec4 {
x: Vec4 { x: m00, y: m01, z: m02, w: m03 },
y: Vec4 { x: m10, y: m11, z: m12, w: m13 },
z: Vec4 { x: m20, y: m21, z: m22, w: m23 },
w: Vec4 { x: m30, y: m31, z: m32, w: m33 },
} = Rows4::from(self).rows;
m03 * m12 * m21 * m30 - m02 * m13 * m21 * m30 -
m03 * m11 * m22 * m30 + m01 * m13 * m22 * m30 +
m02 * m11 * m23 * m30 - m01 * m12 * m23 * m30 -
m03 * m12 * m20 * m31 + m02 * m13 * m20 * m31 +
m03 * m10 * m22 * m31 - m00 * m13 * m22 * m31 -
m02 * m10 * m23 * m31 + m00 * m12 * m23 * m31 +
m03 * m11 * m20 * m32 - m01 * m13 * m20 * m32 -
m03 * m10 * m21 * m32 + m00 * m13 * m21 * m32 +
m01 * m10 * m23 * m32 - m00 * m11 * m23 * m32 -
m02 * m11 * m20 * m33 + m01 * m12 * m20 * m33 +
m02 * m10 * m21 * m33 - m00 * m12 * m21 * m33 -
m01 * m10 * m22 * m33 + m00 * m11 * m22 * m33
}
These were formatted manually such that it's easy to prove, with a glance (espacially for new()), that the order of parameters is correct and there were no mismatches in the function's body either. Whatever rustfmt does in this instance, it certainly won't look as pretty. Don't get me started on determinant()
... x)
Sometimes I also "force" one-liners for the purpose of vertical editing. Take for instance this mess in vec.rs
:
macro_rules! vec_impl_trinop {
(impl $Op:ident for $Vec:ident { $op:tt } ($($namedget:tt)+)) => {
impl< T> $Op< $Vec<T>, $Vec<T>> for $Vec<T> where T: $Op< T, T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, $Vec<T>, $Vec<T>, ($($namedget)+)} }
impl< 'c, T> $Op< $Vec<T>, $Vec<T>> for &'c $Vec<T> where &'c T: $Op< T, T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, $Vec<T>, $Vec<T>, ($($namedget)+)} }
impl< 'b, T> $Op< $Vec<T>, &'b $Vec<T>> for $Vec<T> where T: $Op< T, &'b T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, $Vec<T>, &'b $Vec<T>, ($($namedget)+)} }
impl< 'b, 'c, T> $Op< $Vec<T>, &'b $Vec<T>> for &'c $Vec<T> where &'c T: $Op< T, &'b T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, $Vec<T>, &'b $Vec<T>, ($($namedget)+)} }
impl<'a, T> $Op<&'a $Vec<T>, $Vec<T>> for $Vec<T> where T: $Op<&'a T, T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, &'a $Vec<T>, $Vec<T>, ($($namedget)+)} }
impl<'a, 'c, T> $Op<&'a $Vec<T>, $Vec<T>> for &'c $Vec<T> where &'c T: $Op<&'a T, T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, &'a $Vec<T>, $Vec<T>, ($($namedget)+)} }
impl<'a, 'b, T> $Op<&'a $Vec<T>, &'b $Vec<T>> for $Vec<T> where T: $Op<&'a T, &'b T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, &'a $Vec<T>, &'b $Vec<T>, ($($namedget)+)} }
impl<'a, 'b, 'c, T> $Op<&'a $Vec<T>, &'b $Vec<T>> for &'c $Vec<T> where &'c T: $Op<&'a T, &'b T, Output=T> { vec_impl_trinop_vec_vec!{$op, $Vec<T>, &'a $Vec<T>, &'b $Vec<T>, ($($namedget)+)} }
That's not a pretty sight, right, but blindly applying the rust style guides here would turn "this ugly mess that fits in one screen" into "an ugly mess that DOES NOT fit in one screen so I have to scroll all the time when I want to make a change or proofread". I had written these lines the way they are because it was less painful for me to read and edit via vertical editing in Vim (although they definitely lost their "vertical-editing-friendlyness" somewhere along the way... But the "fit in one screen" point still stands).
I think there's a #[rustfmt_ignore]
attribute or something that I could use in these cases, but there would be too many to warrant the trouble; because for code unlike the pieces I've shown here, I think I've done a pretty good job at following the Rust style guide.
The only things rustfmt would get me are, at worst, minor annoyances such as "oh, this one-line macro call was spread across multiple lines" (or whatever), and at best nothing because the formatting is okay already.
To summarize: I fail to see any clear benefit. However there are some inconsistencies here and there (re: tabs/spaces) which I agree should be fixed.
EDIT: I'd be happy to read about the specific problems in the codebase that caused you to add this issue :)
I think it is #[rustfmt::skip]
if you have updated to 2018 edition.
Just because I think you might find this useful, in VScode I have a section in my settings.json file:
//default maximum line length rustfmt accepts, for referencing
"editor.rulers": [100],
//getting very long lines to wrap around the screen automatically the right way
"editor.wordWrapColumn": 256,
"editor.wordWrap": "bounded",
//prevent breaking the minimap
"editor.minimap.maxColumn": 100,
"editor.minimap.enabled": true,
"editor.minimap.renderCharacters": false,
//there is no reason to not have it next to the explorer and search
"editor.minimap.side": "left",
"editor.minimap.showSlider": "always",
Even if you will always keep the lines below 100, this is still useful while editing and some lines are temporarily very long. If I had my way, there would be no such thing as a maximum line length anywhere in coding, IDEs and whatever you use to view code would nicely wrap comment lines around to whatever your window allows.
Thank you for clarifying!
Use rustfmt to make the code more consistent and nice.