DavidAnson / markdownlint

A Node.js style checker and lint tool for Markdown/CommonMark files.
MIT License
4.83k stars 733 forks source link

MD046 assumes block indents are bad if they are a part of lists. #327

Closed jamesharris-garmin closed 4 years ago

jamesharris-garmin commented 4 years ago

Description

MD046 incorectly flags all fenced code blocks as violations even if they are a part of lists.

Motivating example:

1. Here is my list item

    ```bash
    # example code
  1. Here is my next list item.
    
    which I expect to render as follows:

  1. Here is my list item

    # example code
  2. Here is my next list item.

The vscode markdownlint extension flags this as a violation of MD046 but if I follow the advice on the MD046 wiki page then I get the following rendering:

Example following MD046 advice:

1. Here is my list item

```bash
# example code
``` <!-- space added here to prevent rendering errors in comment-->

2. Here is my next list item.

Rendered result:


  1. Here is my list item
# example code
  1. Here is my next list item.

which is not how I want the list to appear in its rendered form.

Possible Resolutions:

It seems like there are two approaches that could be used to fix this issue:

  1. broaden MD046 to allow for these cases when in lists. (both formations are valid so it probably just needs to be disabled when immediately following list items (ordered or unordered)
  2. Allow the visual studio code extension to ignore specific violations.
DavidAnson commented 4 years ago

Here is your motivating example. It does not seem to trigger a violation of MD046. Could you please clarify the scenario?

https://dlaa.me/markdownlint/#%25m%23%20Issue%20327%0A%0A1.%20Here%20is%20my%20list%20item%0A%0A%20%20%20%20%60%60%60bash%0A%20%20%20%20%23%20example%20code%0A%20%20%20%20%60%60%60%0A2.%20Here%20is%20my%20next%20list%20item.%0A

Regarding your desire to suppress individual violations, that is possible today and documented here: https://github.com/DavidAnson/vscode-markdownlint#suppress

DavidAnson commented 4 years ago

Maybe it's helpful to clarify the intent of rule MD046. It is a stylistic rule to enforce the same technique (indenting or fencing) throughout the document. By default, it assumes the first style it encounters is correct and flags any differences in the rest of the document. In your case, if you have an indented code block earlier in the document, then the fenced code you show above is a violation because it is not an indented code block

MichaIng commented 4 years ago

Interesting, we faced the same issue, but when copying our code into the demo editor, the violation is not shown: Failed check: https://github.com/MichaIng/DietPi-Docs/runs/1272589173?check_suite_focus=true#step:6:5 Code: https://raw.githubusercontent.com/MichaIng/DietPi-Docs/c4a9836a43efc386ff9c6ae1d5f61c6a439ca5d4/docs/release-notes.md

Copying these lines into the demo editor does not throw any violation, compared to the failed test in GitHub action, not sure what the difference is:

# DietPi Updates

## October 2020 (version 6.33)

### Highlights

- **Bazarr** is the latest application from DietPi optimised software portfolio. It is a companion application to Sonarr and Radarr, and manages and downloads subtitles based on defined requirements.

    ![DietPi-Software Bazarr](assets/images/dietpi-software_bazarr.jpg)
    For more details on installation and configuration open [DietPi Optimised Software - Bazarr](../software/bittorrent#bazarr) page.

    Companion application to Sonarr and Radarr, which manages and downloads subtitles based on your requirements, now available for install. Open [Bazarr](../software/bittorrent#bazarr) page in [Optimised software](../software/).
    Many thanks to @DiogoAbu for doing this suggestion [Issue #2045](https://github.com/MichaIng/DietPi/issues/2045)

- **Docker logging available in RAM/journald**

    This feature is now available to newly fresh Docker installs or reinstalls. Logs are now done with limited verbosity to systemd-journald (RAM). They could be accessible running next command:

    ```bash
    journalctl -u docker -u containerd
This change brings the advantage that reduces the disk writes, as well as make them accessible to system journal (external to containerized environment). To fully benefit from this improvment, Docker can be reinstalled using next command:

```bash
dietpi-software reinstall 162
```

Many thanks to @SaturnusDJ for doing this suggestion: [Issue #2388](https://github.com/MichaIng/DietPi/issues/2388)


And to clarify: Removing one or two spaces from list paragraph indentation resolves the violation (but breaks correct HTML conversion), so there is not any case where four spaces are used for code blocks but only these cases for indented code blocks to make them part of the list item.

Also the result of the check says "`[Expected: fenced; Actual: indented]`", which shows that fanced code block were used before and that it detects these two cases not as fanced code blocks but falsely as code blocks via 4-space indentation, instead of recognising it as part of the list syntax.
DavidAnson commented 4 years ago

@MichaIng Your failing test run reports an issue with docs/dietpi_tools.md, but your example and link are for a different file.

Looking at the reported file, we see code fences on lines 7 and 17, so the report of an indented code block on line 27 being inconsistent is correct.

https://github.com/MichaIng/DietPi-Docs/blame/b9ac99eab398f0b850eac7460ea8267f06bf0c4e/docs/dietpi_tools.md

MichaIng commented 4 years ago

Ah indeed, sorry for mixing this up. This makes all sense now, the failing code lines are due to pymdownx.tabbed Markdown extension for content tabs. I guess adding support for this would require a switch/parameter since it is relatively rarely used compared to nested fences via pymdownx.superfences. Was this already a topic once or shall I open a request for it?

Related to the original topic I can hence confirm that nested fences via four-space indentation is correctly handled by markdownlint.

DavidAnson commented 4 years ago

Thanks, I will close this issue. Regarding tabbed behavior, that syntax seems to be a violation of CommonMark syntax, so it's not clear to me how to do better unless there is a markdown-it parser to support that. You're welcome to open an issue, but FYI that there are complications.

MichaIng commented 4 years ago

Many thanks for mentioning CommonMark, good to know there is such a specification, only sad that Python-Markdown core + standard library extensions do not fulfil it completely (e.g. fenced code in list items is not possible) so that pymdownx needs to be used for that, while other extensions of pymdownx are not part of CommonMark. Not a real issue but that makes it difficult for newcomers to differentiate between common/specified syntax that is usually interpreted correctly and uncommon syntax where one has to expect failures of code quality/syntax tests 😅.

But all fine, I see that these content tabs a very uncommon and conflict with some header and thematic break syntax. So the effort to implement support is likely too large effort to be reasonable.