-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use rustfmt #16
Comments
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 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). The biggest ones are in 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 Sometimes I also "force" one-liners for the purpose of vertical editing. Take for instance this mess in 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 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 Just because I think you might find this useful, in VScode I have a section in my settings.json file:
Even if you will always keep the lines below 100, this is still useful while editing and some lines are temporarily very long. |
Thank you for clarifying! |
Use rustfmt to make the code more consistent and nice.
The text was updated successfully, but these errors were encountered: