search

nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
Other
2.06k stars 625 forks source link

Consider formatting markdown according to pre-commit expectations #16852

Closed LeonarddeR closed 3 weeks ago

LeonarddeR commented 1 month ago

cc @seanbudd

Is your feature request related to a problem? Please describe.

With the current pre-commit config, commits including changes.md will perform a major rewrite of the file due to trailing whitespace

Describe the solution you'd like

Format the docs according to pre-commit expectations.

Describe alternatives you've considered

  1. Exclude docs from pre-commit
  2. Disable pre-commit on merge. It seems almost impossible to perform a merge with conflicts without pre-commit kicking in
seanbudd commented 1 month ago

Is this just for trailing white space? Will this diff cover it?

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 5948e88c9..9c24d7333 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -8,6 +8,7 @@ repos:
     - id: check-case-conflict
     - id: check-merge-conflict
     - id: end-of-file-fixer
     - id: trailing-whitespace
 +   exclude: "user_docs/*/*.md"
 - repo: https://github.com/asottile/add-trailing-comma
LeonarddeR commented 1 month ago

We might need to exclude the project docs as well, though i'd rather see that at least the English docs are on par with the trailing spaces rule, since trailing spaces in mark down are discarded anyway. It is likely that it is not only pre-commit conflicting with this rule, but also some ide that enforces removal of trailing whitepsace on remove, for example. I'm happy to provide a fixup pr for this if you prefer.

seanbudd commented 1 month ago

Yep feel free to propose some changes via a PR

LeonarddeR commented 1 month ago

Probably best to wait until #16808 is merged.

seanbudd commented 1 month ago

I think it's worth fixing sooner - I think we want a state where running pre-commit run --all-files passes eventually

seanbudd commented 1 month ago

Disable pre-commit on merge. It seems almost impossible to perform a merge with conflicts without pre-commit kicking in

Workaround in the mean time from git bash SKIP=trailing-whitespace git merge --continue

LeonarddeR commented 1 month ago

I'll try to provide a pr later today that will also fix some small mistakes in the current config.

seanbudd commented 3 weeks ago

Closed by #16936