Skip to content

markdownlint hook's repo-local branch inherits no global rule config, so new rules like MD060 fight terraform-docs output #308

Description

@twistedmelonman

Summary

The markdownlint pre-commit hook defers to a repo-local config when the
target repo has one (#116), which is right. But the global config carries
"MD060": false and the repo-local branch inherits nothing, so a repo whose
config predates MD060 gets the rule at its default. MD060 wants aligned or
compact table pipes; terraform-docs emits |------|---------|. The two
disagree, and the hook blocks every commit touching a generated README.

Hit this on beacon-biosignals/infra, committing an unrelated Terraform
change that happened to include a docs edit. That repo has 93 READMEs
generated by terraform-docs, and MD060 fails on all of them. Beacon's CI
doesn't run markdownlint at all, so this is purely the local hook.

Reproduction

mkdir md-repro && cd md-repro
printf '%s\n' '<!-- BEGIN_TF_DOCS -->' '## Providers' '' \
  '| Name | Version |' '|------|---------|' '| aws | n/a |' \
  '<!-- END_TF_DOCS -->' > README.md
printf '%s\n' '{ "MD013": false }' > .markdownlint.json

# repo-local branch: MD060 fires
markdownlint --fix README.md

# global branch: clean, because the global config disables MD060
markdownlint --fix --config ~/.config/markdownlint-cli/.markdownlint.json README.md

The same file passes under one branch and fails under the other. markdownlint
v0.49.1.

Note MD060 reports but does not auto-fix this case, so the hook fails without
rewriting. On infra's real config it did rewrite tables elsewhere in the
file (that config sets MD013.tables: false, which changes what other rules
touch), producing | ------ | --------- |. That form is worse than the
failure: terraform-docs reverts it on the next terraform-docs run, so it is
permanent churn against a generated file.

Where

pre-commit/config.yaml:121-136

if [[ "$has_local_config" -eq 1 ]]; then
  markdownlint --fix "$@";
else
  markdownlint --fix --config "$HOME/.config/markdownlint-cli/.markdownlint.json" "$@";
fi

The comment above it (lines 113-118) explains the intent: defer to a repo's own
conventions rather than forcing the personal global config. That intent is
correct. The gap is that "defer to the repo" silently means "and also apply
every rule the repo's config never heard of," including rules added to
markdownlint after that config was written.

Why it's worth fixing

Same shape as #290: a global hook default fighting a repo's actual convention,
with an escape hatch nobody knows to reach for until they've lost time to it.

Three things make it worse than #290:

  • The repo that suffers is not mine. Fixing it the obvious way means committing
    a change to Beacon's shared .markdownlint.yaml to accommodate my local
    tooling, which a reviewer would rightly question on an unrelated PR.
  • It scales with the rule set. Every new rule markdownlint ships lands
    enabled-by-default in every repo-local branch. MD060 is new in the v0.4x
    line; the next one will do this again somewhere else.
  • Generated files are the common case. terraform-docs, swagger-markdown, and
    similar tools emit tables in whatever style they emit, and no amount of
    local reformatting survives regeneration.

Options

  1. Layer the global config under the repo's. Pass both, global first, so
    repo settings win but global opinions about rules the repo never mentions
    still apply. markdownlint-cli2 supports config extension; markdownlint-cli
    may need the merge done in the hook.
  2. Disable MD060 globally in the hook regardless of branch, since
    generated tables are common and the rule is cosmetic.
  3. Skip files containing generator sentinels such as BEGIN_TF_DOCS,
    @generated, or DO NOT EDIT. Correct in principle, and the sentinel list
    never stops growing.
  4. Leave as-is and document it, same as lint-shell.sh's -bn shfmt default fights CI in any repo without an .editorconfig #290 option 4.

Preference is 1. It preserves the #116 intent exactly (the repo still wins on
anything it has an opinion about) while stopping a new upstream rule from
becoming a per-repo emergency. 2 is the one-line version if 1 is more
machinery than it is worth.

Happy to send a PR for whichever you prefer.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions