ndarray
ndarray copied to clipboard
Add rustfmt check to CI
rustfmt is very convenient when writing PRs, but when it's not enforced by CI, the formatting in the project inevitably diverges from the rustfmt style. So, when I run rustfmt to format a PR, lines unrelated to the changes in the PR are reformatted (since those lines don't follow the rustfmt style), and then I have to manually stage only the relevant lines and discard the irrelevant formatting changes. If rustfmt style is enforced by CI, then I can just run rustfmt before submitting a PR and not have to worry about discarding unrelated formatting changes (since there shouldn't be any).
This PR should fail CI right now. Once we've merged the large open PRs, we can run cargo fmt on the whole crate, rerun CI for this PR to make sure it passes, and then merge this PR.
While we certainly need some formatting help sometimes, I think rustfmt makes the wrong decisions too often.
Many good suggestions all in all but the below are not that IMO.
-pub(crate) fn can_index_slice_not_custom<D: Dimension>(data_len: usize, dim: &D)
- -> Result<(), ShapeError>
-{
+pub(crate) fn can_index_slice_not_custom<D: Dimension>(
+ data_len: usize,
+ dim: &D,
+) -> Result<(), ShapeError> {
// Condition 1.
- #[deprecated(note = "This method is hard to use correctly. Use `uninit` instead.",
- since = "0.15.0")]
+ #[deprecated(
+ note = "This method is hard to use correctly. Use `uninit` instead.",
+ since = "0.15.0"
+ )]
/// Create an array with uninitalized elements, shape `shape`.
}
- panic!("swap: index out of bounds for indices {:?} {:?}", index1, index2);
+ panic!(
+ "swap: index out of bounds for indices {:?} {:?}",
+ index1, index2
+ );
}
let strides = unlimited_transmute::<D, D2>(self.strides);
- return Ok(ArrayBase::from_data_ptr(self.data, self.ptr)
- .with_strides_dim(strides, dim));
- } else if D::NDIM == None || D2::NDIM == None { // one is dynamic dim
+ return Ok(
+ ArrayBase::from_data_ptr(self.data, self.ptr).with_strides_dim(strides, dim)
+ );
+ } else if D::NDIM == None || D2::NDIM == None {
+ // one is dynamic dim
// safe because dim, strides are equivalent under a different type
if let Some(dim) = D2::from_dimension(&self.dim) {
All examples are a bit of the same theme - so maybe it can be tuned
That makes sense. I prioritize the usefulness of automatic formatting over the quality of the formatting, but that's just my personal preference.
The various rustfmt configuration options are listed here. I don't see any way to configure rustfmt to preserve the formatting in the first example. For the second example, there are options to preserve this indentation style (called "Visual" in the configuration) for some things (imports, expressions, function args, etc.), but I don't see an existing option like this for attributes. The third example could work by increasing the relevant width option. The fourth example may work with overflow_delimited_expr set to true, but I'm not sure.
My two cents are that the first three examples aren't that bad, but might take some getting used to. The last one isn't very good, but the way to fix it would be to split it into two lines like this:
let base = ArrayBase::from_data_ptr(self.data, self.ptr);
Ok(base.with_strides_dim(strides, dim))
I think the way to go is to use unstable format settings, as soon as rustfmt 2 is out it is also possible on stable.
As long as I can fix example 1 I'm ready to go.
I strongly believe that the value of consistent formatting massively outweighs the pain of small disagreements about the formatting of specific cases.
That said, if you can find a configuration that does what you want, everyone wins