Skip to content
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

feat: the trailingWhitespace linter #920

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

adomani
Copy link
Contributor

@adomani adomani commented Aug 16, 2024

The trailingWhitespace linter is a syntax linter. It emits a warning whenever a line ends with a space or a file does not end with a line break.

Zulip thread

@digama0
Copy link
Member

digama0 commented Aug 16, 2024

I think this should be disabled by default, or only enabled in CI. For people using vscode with the "remove trailing whitespace on save" option, it will only ever be a false positive triggering between keystrokes.

@adomani
Copy link
Contributor Author

adomani commented Aug 16, 2024

That's true: it was not easy to get the test file to work, since I could not save it!

Would the "script-style" code below be better than a linter, then?

import Lean.Elab.Command

open Lean in
run_cmd
  for fil in ← System.FilePath.walkDir ((← IO.currentDir) / "Batteries") do
    if let some "lean" := fil.extension then
      let lines ← IO.FS.lines fil
      for l in [:lines.size] do
        let line := lines[l]!
        let tr := line.takeRightWhile (·.isWhitespace)
        if !tr.isEmpty then
          logWarning
            m!"{fil}:{l} ends with {tr.length} space{if tr.length == 1 then "" else "s"}"
      if (← IO.FS.readFile fil).back != '\n' then
        logWarning m!"{fil} does not end with a line break"

This has the advantage that you do not need to make sure that the linter is imported everywhere and you can run the script on itself as well.

@fgdorais fgdorais added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Sep 25, 2024
@leanprover-community-bot
Copy link
Collaborator

leanprover-community-bot commented Oct 1, 2024

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Oct 2, 2024
@digama0
Copy link
Member

digama0 commented Oct 14, 2024

We don't really have a place to put scripts like this. Ideally we could put it somewhere such that lake would make it available to downstream packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review This PR is ready for review; the author thinks it is ready to be merged. builds-mathlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants