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

docs(manager/gleam): fix readme formatting #30569

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

SnakeDoc
Copy link
Contributor

@SnakeDoc SnakeDoc commented Aug 3, 2024

Changes

Correct a formatting error in the gleam manager readme, caused by prettier, which resulted in the example sub-lists rendering as parent list items.

Current Render:

image

Context

Fix markdown formatting and have prettier ignore the block. Tested using mkdocs locally.

Fixed Render:

image

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@rarkins
Copy link
Collaborator

rarkins commented Aug 3, 2024

Is this a known prettier problem? I dislike needing to have the prettier-ignore permanently

@SnakeDoc
Copy link
Contributor Author

SnakeDoc commented Aug 3, 2024

I was surprised myself, actually. Without the ignore directive, it removes 1 space char in the sub-list (leaving 3 instead of 4 spaces of indentation). It still renders as expected in whatever markdown renderer Github uses as well as my ide, but somehow mkdocs loses it in translation.

@rarkins
Copy link
Collaborator

rarkins commented Aug 3, 2024

Waiting in case @HonkingGoose has any insights

@SnakeDoc
Copy link
Contributor Author

SnakeDoc commented Aug 3, 2024

Looking at it a bit more, it does look like prettier has a default tabWidth of 2 space chars, which seems to cause this issue with embedded sub-lists for a reason I don't yet understand.

When I add "tabWidth": 4 to .prettierrc.json, then the prettier-ignore is unnecessary. However, setting the tab width to anything but default will have a significant impact. Perhaps HonkingGoose will know more.

@HonkingGoose
Copy link
Collaborator

HonkingGoose commented Aug 3, 2024

Summary

We use MkDocs plus the Material for MkDocs extension to build our docs. MkDocs uses Python-Markdown, which mostly matches the Markdown spec.

But Python-Markdown is different in:

  • Middle-Word Emphasis
  • Indentation/Tab Length (MkDocs forces 4 spaces, instead of letting you do whatever)
  • Consecutive Lists (MkDocs ignores list marker changes, which is good as Prettier forces it all to 1. or - anyways)

The problem with setting tabWidth to 4 spaces is that ordered lists seem to need 4 and 2 spaces! At least that's what the Material for MkDocs examples use.

I don't know if setting tabWidth to 4 spaces is better than what we do now. We should try it in a new PR though, to see what changes, and then decide what's best. 😉

MkDocs uses Python-Markdown

Quote from MkDocs, writing with Markdown:

MkDocs uses the Python-Markdown library to render Markdown documents to HTML. Python-Markdown is almost completely compliant with the reference implementation, although there are a few very minor differences.

Differences in Python-Markdown

Quote from Python-Markdown, differences:

Differences

While Python-Markdown strives to fully implement markdown as described in the syntax rules, the rules can be interpreted in different ways and different implementations occasionally vary in their behavior (see the Babelmark FAQ for some examples). Known and intentional differences found in Python-Markdown are summarized below:

Middle-Word Emphasis

Python-Markdown defaults to ignoring middle-word emphasis (and strong emphasis). In other words, some_long_filename.txt will not become somelongfilename.txt. This can be switched off if desired. See the Legacy EM Extension for details.

Indentation/Tab Length

The syntax rules clearly state that when a list item consists of multiple paragraphs, “each subsequent paragraph in a list item must be indented by either 4 spaces or one tab” (emphasis added). However, many implementations do not enforce this rule and allow less than 4 spaces of indentation. The implementers of Python-Markdown consider it a bug to not enforce this rule.

This applies to any block level elements nested in a list, including paragraphs, sub-lists, blockquotes, code blocks, etc. They must always be indented by at least four spaces (or one tab) for each level of nesting.

In the event that one would prefer different behavior, tab_length can be set to whatever length is desired. Be warned however, as this will affect indentation for all aspects of the syntax (including root level code blocks). Alternatively, a third party extension may offer a solution that meets your needs.

Consecutive Lists

While the syntax rules are not clear on this, many implementations (including the original) do not end one list and start a second list when the list marker (asterisks, pluses, hyphens, and numbers) changes. For consistency, Python-Markdown maintains the same behavior with no plans to change in the foreseeable future. That said, the Sane List Extension is available to provide a less surprising behavior.

Material for MkDocs reference guide, lists

Examples from the Material for MkDocs, reference, lists.

Ordered lists

Syntax:

  • Start bullet at position 0
  • Sub list uses 4 spaces
- Nulla et rhoncus turpis. Mauris ultricies elementum leo. Duis efficitur
  accumsan nibh eu mattis. Vivamus tempus velit eros, porttitor placerat nibh
  lacinia sed. Aenean in finibus diam.

    * Duis mollis est eget nibh volutpat, fermentum aliquet dui mollis.
    * Nam vulputate tincidunt fringilla.
    * Nullam dignissim ultrices urna non auctor.

Ordered lists

Syntax:

  1. Start with list marker 1. at position 0
  2. Sub-list starts with 4 spaces then the 1. list marker, then 2 spaces, then the list text
1.  Vivamus id mi enim. Integer id turpis sapien. Ut condimentum lobortis
    sagittis. Aliquam purus tellus, faucibus eget urna at, iaculis venenatis
    nulla. Vivamus a pharetra leo.

    1.  Vivamus venenatis porttitor tortor sit amet rutrum. Pellentesque aliquet
        quam enim, eu volutpat urna rutrum a. Nam vehicula nunc mauris, a
        ultricies libero efficitur sed.

    2.  Morbi eget dapibus felis. Vivamus venenatis porttitor tortor sit amet
        rutrum. Pellentesque aliquet quam enim, eu volutpat urna rutrum a.

        1.  Mauris dictum mi lacus
        2.  Ut sit amet placerat ante
        3.  Suspendisse ac eros arcu

@SnakeDoc SnakeDoc mentioned this pull request Aug 6, 2024
6 tasks
@SnakeDoc
Copy link
Contributor Author

While #30608 continues to be worked on as an overall fix for this issue (and general formatting update for all markdown), would it be acceptable to merge this PR in the meantime?

I can add a comment to the ignore directive and point to this issue as a reminder.

There is at least one other doc where this same issue occurs (the conan manager). Perhaps we should close this PR, and open a new PR that addresses each instance of this issue (adding the same prettier-ignore + comment to each)?

@viceice viceice changed the title docs: fix gleam manager readme formatting docs(manager/gleam): fix readme formatting Aug 16, 2024
@viceice viceice added this pull request to the merge queue Aug 16, 2024
Merged via the queue into renovatebot:main with commit a3ceda9 Aug 16, 2024
38 checks passed
@SnakeDoc SnakeDoc deleted the docs/gleam-doc-formatting branch August 16, 2024 06:04
@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 38.37.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

kosmoz pushed a commit to kosmoz/renovate that referenced this pull request Aug 16, 2024
@HonkingGoose
Copy link
Collaborator

I'm happy to merge this PR, which @viceice already did. 🙃 I suspect we'll be stuck on the proper fix for a while.

I can add a comment to the ignore directive and point to this issue as a reminder.

Sound good, but you should link to the "proper fix PR" instead. 😉

There is at least one other doc where this same issue occurs (the conan manager).

Do you want to create a PR to apply the same "hack-job fix" to that document?

@SnakeDoc
Copy link
Contributor Author

Yes, I'll work on applying this "hack" to the conan manager, and will do my best to locate any other occurrences while we're at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants