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

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

Open
fedy-cz opened this issue Nov 1, 2023 · 10 comments
Open

Comments

@fedy-cz
Copy link

fedy-cz commented Nov 1, 2023

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:

  • It's not the default so by the time I realize it's happening I might have lost quite a lot of work
  • I like to have the lint active in the editor (as a reminder). Just don't fix it automatically like the other ones.

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:

  • ignore
  • hint
  • auto-fix

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

@DavidAnson
Copy link
Owner

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: DavidAnson/markdownlint#254

@fedy-cz fedy-cz changed the title MD053 shouldn't get auto-fixed on save be default (loosing useful content) MD053 shouldn't get auto-fixed on save by default (loosing useful content) Nov 1, 2023
@fedy-cz
Copy link
Author

fedy-cz commented Nov 1, 2023

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
Copy link
Owner

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
Copy link
Author

fedy-cz commented Nov 1, 2023

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
Copy link
Owner

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
Copy link
Author

fedy-cz commented Nov 29, 2023

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
Copy link
Author

fedy-cz commented Nov 29, 2023

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):

  • WARN_AND_PASS - produces warnings, but doesn't affect if the document is passed or rejected
  • WARD_AND_REJECT - produces warnings (to be manually fixed) and rejects to pass documents with this issue

@DavidAnson
Copy link
Owner

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
Copy link

Gozala commented Mar 22, 2024

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
Copy link
Owner

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.

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