DavidAnson / vscode-markdownlint

Markdown linting and style checking for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint
MIT License
894 stars 166 forks source link

MD053 shouldn't get auto-fixed on save by default (loosing useful content) #306

Open fedy-cz opened 8 months ago

fedy-cz commented 8 months ago

TLDR: Lint MD053 Link and image reference definitions should be needed should not be automatic.

Explanation:

While most of the linting rules are probably safe to do automatically "on save", MD053 probably isn't one of them. It breaks (I think a pretty common) workflow:

  1. Build up a list of useful ref-links that I will use later in the document
  2. Write the document itself using the links prepared before

The gotcha: If I save at any point between 1. and 2. I loose all my ref-links that I didn't use yet.

I can disable the lint completely in the config, but:

As a general rule:

Any lint that could potentially remove useful content should not be applied automatically (at least not by default).

It could be nice to be able to configure each lint at 3 possible levels:

But the important think is to have reasonable defaults that don't delete potentially useful content automatically.

DavidAnson commented 8 months ago

Lint-on-save is not the default, it is something you have opted into. Here is the documentation: https://github.com/DavidAnson/vscode-markdownlint#fix

Something you may find useful is the ability to quickly quickly toggle linting on and off: https://github.com/DavidAnson/vscode-markdownlint#disable

The suggestion to add warning/error as distinct states for each rule is tracked in the library repository here: https://github.com/DavidAnson/markdownlint/issues/254

fedy-cz commented 8 months ago

Ok.

I still think it could be a good idea to distinguish between cosmetic and "potentially destructive" lints. Maybe it's just incorrect configuration on my side, but I find it kind of weird when lints like line wrapping (which could be easily automated) need to be fixed manually, but unused links just get deleted without warning.

Speaking of line-wraps: I noticed that MD013 gets triggered for preformatted text / code blocks too.

DavidAnson commented 8 months ago

MD013 can be configured to ignore code blocks or apply a different length to them: https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md

fedy-cz commented 8 months ago

Lint-on-save is not the default, it is something you have opted into. Here is the documentation: https://github.com/DavidAnson/vscode-markdownlint#fix

Regarding this: I don't have it enabled for markdownlint specifically but globally. It didn't cause any issues with other code linters so far.

DavidAnson commented 7 months ago

The proposal for a per-rule setting to ignore/report/fix (with fix being the new option) is interesting. I think it only applies to the interactive editor scenario, though, because that's the only place where the "temporarily wrong but on purpose" scenario seems to make sense. Because of that, the configuration for this would belong in VS Code Settings instead of a .markdownlint* file. But even @fedy-cz probably doesn't want to disable fix of MD053 violations ALL the time - just some of the time when they've deliberately added new link reference definitions. Which means putting this in a setting is awkward unless that setting is easily toggled. Which starts to sound a lot like the markdownlint.toggleLinting command that already exists... So I'm not seeing a clean way to offer this that's easily understood and used.

fedy-cz commented 7 months ago

I think it only applies to the interactive editor scenario, though, because that's the only place where the "temporarily wrong but on purpose" scenario seems to make sense.

I don't think the distinction is about temporality of the detected issue. It's about whether the fix should be automatic or performed manually by the user. Makes sense to me for non-interactive use as well:

  1. Run the linter
  2. It spits out bunch of warnings (and auto-fixes some other issues)
  3. Edit the document and (potentially) fix some of the warnings
  4. GOTO 1
fedy-cz commented 7 months ago

And for "validation" runs (e.g. automatically deciding if the document should pass into production) the "warning" level could be split into two sub-levels (that would affect linters exit code or other means to signalize the binary pass/fail state):

DavidAnson commented 7 months ago

There's a separate issue for separate warning versus error level. We can ignore that here. To my point above about this being an editor scenario only, people using the CLI can choose not to fix at all or can choose to fix just a subset of issues based on the configuration they pass in. All of that is already possible today. What seems unique about your scenario is that you temporarily want to allow issues as a reminder to address them.

Gozala commented 3 months ago

Is there way to disable MD053 somehow ? I keep loosing links because typos or while cut & pasting things within vscode which is really annoying. I tried to set following vscode settings.json but this does not seem to work

"markdownlint.config": {
    "MD053":false
}

I also fully agree with @fedy-cz, I'd love vscode do same thing it does for MD051/link-fragments, that is warn be about unreferenced link with a squiggly line but I really don't want it to just delete those links.

DavidAnson commented 3 months ago

There are various ways to disable rules, they should be covered here: https://github.com/DavidAnson/vscode-markdownlint?tab=readme-ov-file#markdownlintconfig

If what you tried above didn't work, you may need to use a mechanism with higher precedence.