remarkjs / remark-lint

plugins to check (lint) markdown code style
https://remark.js.org
MIT License
938 stars 129 forks source link

add `.pre-commit-hooks.yaml` for those using this tool #227

Closed wasdee closed 4 years ago

wasdee commented 4 years ago

https://pre-commit.com/hooks.html is missing this cool tool. 😞

Can we add the .pre-commit-hooks.yaml?

ChristianMurphy commented 4 years ago

Hey @CircleOnCircles! :wave: Thanks for the suggestion!

When you say "it is missing", do you mean:

wasdee commented 4 years ago

You'd l#ike documentation on how pre-commit could be used to run remark-lint?

More like this one.

There are 2 jobs

  1. Update pre-commit repo that remark-lint is ready for pre-commit
  2. Add a new file to this repo or the cli repo, the yaml above. So that pre-commit know how to expose and handle this tool correctly.
ChristianMurphy commented 4 years ago

Thanks for the clarification: A few options:

  1. If you're up for it, you could open a PR to https://github.com/remarkjs/remark (where the CLI lives) and we could see if this would be a good fit.
  2. This could be added as a guide similar to https://github.com/unifiedjs/unifiedjs.github.io/issues/7, so this could be used for mdx, rehype, retext, and redot as well,
  3. pre-commit isn't the only git hooks framework, https://github.com/typicode/husky is another, husky is able to leverage CLI's provided by node_modules automatically, which can make integrating remark a lot easier.
wasdee commented 4 years ago

Thanks for your advice. I will try to do a PR soon. I come from a Python domain, thus pre-commit seems more appealing to me. I will definitely check out the husky.

wooorm commented 4 years ago

I’m all open to new integrations. Typically, they can be maintained on their own, outside of this project. In this case, as that’s a Python project and I don’t speak Python, it doesn’t sit too well with me to maintainer it here.

A couple of other integrations are listed here in the readme: https://github.com/remarkjs/remark-lint#integrations

So, if someone wants to add this, that’s welcome!

henryiii commented 3 years ago

pre-commit is a general purpose linting framework that happens to be written in Python. It supports a host of different languages. With pre-commit.ci, there is quite a nice online tool for running the checks and updates. It's ideal if the pre-commit-hooks.yaml (which is a very small, simple file) can be in the main repository, because pre-commit caches on the revision. It has been done in a few special cases, but it either requires manually or automatically syncing tags on releases. Dropping this file into the main repo is much easier to maintain. However, on quick testing, it seems the "package.json" is missing the name/version, which are required by npm install ..

Setting this up as a local hook works, but it seems that "warnings" do not trigger a "failing" exit code, so pre-commit thinks the hooks pass.

Here's my local config:

- repo: local
  hooks:
  - id: remarklint
    name: remarklint
    language: node
    entry: remark
    types: [markdown]
    args: ["-u", "preset-lint-recommended"]
    verbose: true  # Necessary to see if this is working at all since this always "passes"
    additional_dependencies: [remark, remark-lint, remark-cli, remark-preset-lint-recommended]

PS: changing checks are even better, but I wasn't sure how to run the changing one from the CLI.

wooorm commented 3 years ago

You can pass --frail as an arg to remark. That will result in warnings being seen as errors

henryiii commented 3 years ago

Thanks! I have this now:

- repo: local
  hooks:
  - id: remarklint
    name: remarklint
    language: node
    entry: remark
    types: [markdown]
    args: ["--frail", "--quiet"]
    additional_dependencies: [remark, remark-lint, remark-cli, remark-preset-lint-recommended, remark-lint-list-item-indent]

This works pretty well, though admittedly, I don't understand why I have to explicitly list remark-lint-list-item-indent to configure it in my .remarkrc file if it's included by remark-preset-lint-recommended.

To make this easier, a little repo could be made with a hook file with more-or-less the contents above, though it probably should be a node hook and the dependencies specified through package.json (so users could add ones they needed, like preset-lint-recommended or remark-lint-list-item-indent without having to specify remark and such over again). Also the versions should be pinned (at least remark-lint).

Would you be interested in keeping something like that in this org? It would need version bumps, but that should be the only maintenance needed.

henryiii commented 3 years ago

PS: the benefit, other than making it easier for pre-commit users to add this, and being able to be listed on https://pre-commit.com/hooks.html, would be that pre-commit.ci and pre-commit autoupdate would be able to keep this hook up-to-date like all the other non-local hooks. See https://github.com/cheshirekow/cmake-format-precommit for an example of a stand-alone repo alongside a normal repo (https://github.com/cheshirekow/cmake-format).

ChristianMurphy commented 3 years ago

I'm not against pre-commit support for remark-lint, I'm not sure I'm for it either. I use pre-commit myself on the occasional Python project. There are some considerations to this integration living in the remark-lint repo which make me cautious:

  1. Would adding a YAML file to this repository offer a signifantly better experience than adding a recipe showing how to do the setup to https://unifiedjs.com/learn/ ?
  2. Most of remark-lints' maintainers don't use use pre-commit, a few use it on occasion. How would it be regularly tested to ensure it continues working?
  3. The automation in this repository is npm and github-actions based, is there a way to handle the updates to the config and do automated testing on the configs from these toolchains? (I'm not seeing it documented at https://pre-commit.com/#new-hooks and https://github.com/pre-commit/action is deprecated)
  4. Who would handle answering questions and issue triage with the integration? Would you or someone you know be interested handling the support aspect that goes along with the integration?
henryiii commented 3 years ago

Well, a first step might be just showing the example I have above somewhere, it does work, though it would be nice to offer an actual integration.

  1. Still not sure why everyone ties pre-commit to Python. I'm using it on a C++ project currently, and I also use it on Ruby projects.
  2. I think the correct direction is to make a remarkjs/remark-lint-pre-commit repository with a light weight package.json + .pre-commit-hooks.yaml.
  3. It should be easy to check in CI. There are a quite a few checks in the list of known hooks that check themselves.
  4. You don't need pre-commit/action; you can just add - run: pipx run pre-commit run -a on any GHA action file, no setup needed.
  5. I'd be available for a bit of help with the pre-commit side of things. I'm not very knowledgeable on the Node side, though.

I've played with setting this up, but ran into a node issue. What I'd like is to have a package.json file like this:

{
  "name": "required-but-not-used",
  "version": "0.0.0",
  "dependencies": {
    "remark-lint": "8.0.0",
    "remark": "*",
    "remark-cli": "*"
  },

And then be able to access remark somehow. But I have no idea how to run from the correct bin folder; I think it gets nested inside the requirements somewhere. Pre-commit is preparing the environment like this:

npm install --dev --prod --ignore-prepublish --no-progress --no-save
npm pack <prefix> # produces name of pkg
npm install -g <pkg>

Would you happen to know how I can add a requirement to package.json and then "export" a script from it to be used? I've tried a few things like "scripts": {"remark": "remark"}, but I can't get the "nested" requirements to show up in the "global" location in that prefix.

ChristianMurphy commented 3 years ago

Still not sure why everyone ties pre-commit to Python. I'm using it on a C++ project currently, and I also use it on Ruby projects

Because git hooks themselves aren't terribly complex, each language ecosystem adds abstractions around hooks tuned for that language's own tools to add value above and beyond plain hooks.

It's true that any of the above could be used with any other language, and it's perfectly fine to do so. But each is optimized/tuned for the language and ecosystem it is written in, for pre-commit that happens to be Python.

I think the correct direction is to make a remarkjs/remark-lint-pre-commit repository with a light weight package.json + .pre-commit-hooks.yaml.

Thoughts on this @remarkjs/core and @remarkjs/maintainers?

It should be easy to check in CI. There are a quite a few checks in the list of known hooks that check themselves.

:+1:

You don't need pre-commit/action; you can just add - run: pipx run pre-commit run -a on any GHA action file

:+1:

no setup needed.

* except for this script and setting up Python

I'd be available for a bit of help with the pre-commit side of things

Thanks! :bow:

But I have no idea how to run from the correct bin folder

npx (https://docs.npmjs.com/cli/v7/commands/npx) can automatically find and run scripts in the bin folder. Or it can be manually accessed as it will be consistently located relative to the project root in ./node_modules/.bin/remark after npm install is run

henryiii commented 3 years ago

Because git hooks themselves aren't terribly complex, each language ecosystem adds abstractions around hooks

pre-commit is a linter runner, more than just a githooks framework (which it also is - but you don't need to run pre-commit install, I often just use pre-commit run -a). It supports linters in many languages natively, automatically handling environment setup, etc, for each one. Ruby's overcommit is very much a hook manager; any extra language support is just basically running shell scripts. Husky also seems to be very JS focused. While pre-commit supports conda, coursier, docker, docker_image, dotnet, fail, golang, node, perl, python, python_venv, r, ruby, rust, swift, pygrep, script, or system. It's obviously strong in Python with a few repeats there, and you are promised a working version of Python when running it since it's written in Python, but otherwise, it's quite general, and you don't really have to touch Python or manual shell commands. Now that it has its own CI service, you really don't have to touch Python to use it. I use it in C++ and Ruby projects (besides, of course, Python ones).

except for this script and setting up Python

Well, pipx run pre-commit run -a is a bash script, though I'd concede that pipx is a Python thing. You do not need setup-python, however; in fact, it will do nothing. pipx is a supported stand-alone package manager on all GitHub runners (and in fact, it's used to setup up a few of the other built in tools and is used in some actions).

Thanks for the pointers, I'll tinker on this and get back to you.

ChristianMurphy commented 3 years ago

pre-commit is a linter runner

shell is a lint runner too :wink: :slightly_smiling_face: It supports tools in all the languages you mentioned and more.

it's quite general

Agreed, and that's part of its niche.

Many languages have comprehensive, integrated, and opinionated package management and script/test/lint runners. Node has NPM and Webpack (among others), Java has Maven and Gradle, Python has Poetry, Ruby has Gem and Rake, etc. Most projects either have: a single language, single package manager, and single build tool, or are a monorepo with several subprojects each with its own language, package manager, and build tool. In that context, developers sometimes/often prefer commit hooks and linting that is tied into the tool chain they are using.

pre-commit fits in well with Python projects, or with projects that don't have an established tool chain.

I use it in C++ and Ruby projects (besides, of course, Python ones).

I don't know why you are taking this as a personal affront. Again, it's fine to use pre-commit on projects, Python or not, it's a wonderful tool. :heart:

There are also other wonderful tools, the tools listed above https://github.com/remarkjs/remark-lint/issues/227#issuecomment-883774976, not to mention SonarQube, and host of other code quality tools. Each have their own niche, pre-commit's is primarily git hooks for Python.

muppetjones commented 7 months ago

Came looking for this integration specifically. Without it, I'll probably just fall back to prettier. I wouldn't say it's critical, but markdown is annoying enough to deal with without having to deal with linter compatibility. This is personal preference, of course.

ChristianMurphy commented 7 months ago

@muppetjones it helps to read discussion before commenting. 🙂 In particular see https://github.com/remarkjs/remark-lint/issues/227#issuecomment-678647783 and https://github.com/remarkjs/remark-lint/issues/227#issuecomment-882747643

If you want to get started quickly:

- repo: local
  hooks:
  - id: remarklint
    name: remarklint
    language: node
    entry: remark
    types: [markdown]
    args: ["--frail", "--quiet"]
    additional_dependencies: [remark, remark-lint, remark-cli, remark-preset-lint-recommended]

If you'd like to build a mirror with something more specific/custom, feel free to https://github.com/pre-commit/pre-commit-mirror-maker and that can be shared back with the community an integration linked in the documentation.

I'm locking this issue as this is not a planned code change in this repo, and the issue tracker is a place to track changes for this repository specific, rather than questions or discussions.

If folks have questions on using remark-lint the Q&A discussion board is open. https://github.com/orgs/remarkjs/discussions