[Merged by Bors] - chore: miscellaneous linter whitespace fixes#30663
[Merged by Bors] - chore: miscellaneous linter whitespace fixes#30663grunweg wants to merge 1 commit intoleanprover-community:masterfrom
Conversation
PR summary 3af1c0aa0eImport changes for modified filesNo significant changes to the import graph Import changes for all files
Declarations diff
You can run this locally as follows## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>
## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>The doc-module for No changes to technical debt.You can run this locally as
|
|
@YaelDillies @Ruben-VandeVelde Would one of you like to review this one? |
YaelDillies
left a comment
There was a problem hiding this comment.
Easy!
maintainer merge
| ∀ (n i : ℕ), i < n → (iterate f a n)[i]? = f^[i] a | ||
| | n + 1, 0 , _ => by simp | ||
| | n + 1, 0, _ => by simp | ||
| | n + 1, i + 1, h => by simp [getElem?_iterate f (f a) n i (by simpa using h)] |
There was a problem hiding this comment.
This one is more subtle I think, there was clear intent to align the =>. I think it's okay to change it, but IMO this should be allowed by whatever tooling we use.
There was a problem hiding this comment.
I agree with the intent. I'm sceptical about manual alignment, though - as this often becomes stale.
In this case, the proofs are also similar, but not too parallel, so this wasn't worth it to me.
I'm not sure how easy or difficult manual alignment is to detect. Manually disabling the linter seems like a good enough band-aid for now.
|
|
||
| theorem formPerm_apply_lt_getElem (xs : List α) (h : Nodup xs) (n : ℕ) (hn : n + 1 < xs.length) : | ||
| formPerm xs xs[n] = xs[n+1] := by | ||
| formPerm xs xs[n] = xs[n + 1] := by |
There was a problem hiding this comment.
These ones are also debatable IMO. I like a bit of flexibility in sticking together things n+1 in a complicated expression. But I won't fight for it.
There was a problem hiding this comment.
I understand this, abstractly! I think, however, that doing so consistently in a library like mathlib is very hard to impossible, and would much rather have a consistent and enforceable rule. (It's not trivial to make the commandStart linter allow this. Maybe the autoformatter will, but we won't know that for a while.)
ADedecker
left a comment
There was a problem hiding this comment.
Thanks! You can ignore my comments, they are just things I wanted to nuance.
bors d+
|
✌️ grunweg can now approve this pull request. To approve and merge a pull request, simply reply with |
|
Thanks for the careful review. I agree both places you flagged could be discussed carefully in the abstract. In this case, I think both changes are fine, so let me go ahead with the one that's easier to automate (namely, making the linter happy). |
Extracted from #30658. Found by `adomani`'s extension of the commandStart linter to proof bodies.
|
Pull request successfully merged into master. Build succeeded: |
) Extracted from leanprover-community#30658. Found by `adomani`'s extension of the commandStart linter to proof bodies.
) Extracted from leanprover-community#30658. Found by `adomani`'s extension of the commandStart linter to proof bodies.
Extracted from #30658. Found by
adomani's extension of the commandStart linter to proof bodies.