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

Adding markdownlint-fix to precommit config? #233

Closed
rrauenza opened this issue Nov 9, 2021 · 12 comments
Closed

Adding markdownlint-fix to precommit config? #233

rrauenza opened this issue Nov 9, 2021 · 12 comments

Comments

@rrauenza
Copy link
Contributor

rrauenza commented Nov 9, 2021

Do you think it would be worth adding another hook for --fix?

I can do a pull request, but it's also a simple change I think. Just checking in first on your thoughts...

@DavidAnson
Copy link
Collaborator

If pre-commit supports parameters, I would suggest that, I think. Else, another hook seems reasonable.

@janosh
Copy link
Contributor

janosh commented Nov 13, 2021

See https://pre-commit.com/#config-args.

Simple as

- repo: local
  hooks:
    - id: some-hook
      args: [--first-arg, --second-arg]

@rrauenza
Copy link
Contributor Author

I'd rather not use "local" so I can pin the repo via precommit and keep it in sync with the check mode...

@janosh
Copy link
Contributor

janosh commented Nov 16, 2021

@rrauenza I think you misunderstood. My example was just to demonstrate how to pass command line arguments to a hook. Put whatever you like for the repo.

@DavidAnson
Copy link
Collaborator

Are there concerns with #235 ? My limited understanding of pre-commit suggested it was a reasonable change.

@janosh
Copy link
Contributor

janosh commented Nov 16, 2021

@DavidAnson I don't see the need for #235. People who want to run with the --fix flag can can already do so like this:

  - repo: https://github.com/igorshubovych/markdownlint-cli
    rev: v0.29.0
    hooks:
      - id: markdownlint
        args: [--fix]

@DavidAnson
Copy link
Collaborator

And now they could do that by passing name "markdownlint-fix" and no args, right? I agree this is not necessary and I am normally against duplication, but I think it makes the idea of fixing during precommit more discoverable. I'm not expecting a lot of maintenance cost, so I'm inclined to leave it.

@DavidAnson
Copy link
Collaborator

Fixed by 2d2ba47

@janosh
Copy link
Contributor

janosh commented Nov 16, 2021

And now they could do that by passing name "markdownlint-fix" and no args, right? I agree this is not necessary and I am normally against duplication, but I think it makes the idea of fixing during precommit more discoverable. I'm not expecting a lot of maintenance cost, so I'm inclined to leave it.

That's right. I agree, little extra work from having two hooks.

@rrauenza
Copy link
Contributor Author

@DavidAnson I don't see the need for #235. People who want to run with the --fix flag can can already do so like this:

  - repo: https://github.com/igorshubovych/markdownlint-cli
    rev: v0.29.0
    hooks:
      - id: markdownlint
        args: [--fix]

This doesn't work if you want to have both in the precommit file -- a fix and a check... the id's would duplicate.

I was assuming not every lint error could be fixed.

Maybe having two is redundant... I didn't look to see how --fix works when / if there are errors it can't fix.

Is this actually redundant in my precommit config?

  - repo: https://github.com/rrauenza/markdownlint-cli
    rev: 68b557874fcdd9aa19d0872180268c3b1b38053c
    hooks:
       - id: markdownlint-fix
       - id: markdownlint

@janosh
Copy link
Contributor

janosh commented Nov 16, 2021

This doesn't work if you want to have both in the precommit file -- a fix and a check... the id's would duplicate.

Why do you need both? markdownlint should complain about stuff it can't fix so it still acts like a checker in fix mode. Correct me if I'm wrong, this was just my assumption when I replied above.

@DavidAnson
Copy link
Collaborator

That's right, fix will do what it can and report on the things that remain. However, some people may only want to scan and not fix and so I think having both entries is reasonable so they can use the one that is best suited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants