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

Semantic line feed in bullets leading to panic with prettier #4

Closed
jamesbraza opened this issue Jun 26, 2023 · 15 comments
Closed

Semantic line feed in bullets leading to panic with prettier #4

jamesbraza opened this issue Jun 26, 2023 · 15 comments

Comments

@jamesbraza
Copy link

jamesbraza commented Jun 26, 2023

I am using mdformat==0.7.16, mdformat-mkdocs==1.0.1, and prettier==3.0.0-alpha.9-for-vscode with tabWidth = 4 to match PEP 8.

Here is the input, and this is fine for mdformat and prettier:

1. Semantic line feed where the following line is
   three spaces deep

However, after adding mdformat-mkdocs, there is a panic about the "three spaces deep" part. mdformat-mkdocs wants to add an extra space before "three" so it's 4-spaces deep:

1. Semantic line feed where the following line is
    three spaces deep

I think this is a bug with mdformat-mkdocs, imo it shouldn't touch the semantic line feed's spacing.

@KyleKing
Copy link
Owner

KyleKing commented Jun 26, 2023

Hi @jamesbraza, thanks for trying out mdformat-mkdocs!

I think a 3-space indent on a numbered list is either an intentional choice or a bug in prettier. I found one related issue that requested the behavior that mdformat-mkdocs implements: prettier/prettier#5635

Is the 3-space indent a standard and/or documented somewhere?


Semi-relatedly, I have run into issues with prettier not properly supporting 4-space indents like prettier/prettier#5019, which is why I've used prettier and mdformat separately: https://github.com/KyleKing/calcipy_template/blob/8a67593aae13d2ea88d3981bd9c53479f1b65c16/.pre-commit-config.yaml#L51-L72

And TIL about https://github.com/sembr/specification

@jamesbraza
Copy link
Author

jamesbraza commented Jun 27, 2023

The 3-space indent on a semantic line feed gets enforced by mdformat without mdformat-mkdocs, and prettier. Try running mdformat (without mdformat-mkdocs):

1. Semantic line feed where the following line is
    three spaces deep

You'll see that second line gets corrected to 3 spaces. Check this comment for why.

I think mdformat-mkdocs should tab over sub-bullets to 4 spaces, but should not tab over semantic line feeds to 4 spaces

@KyleKing
Copy link
Owner

Oh okay, let me take a look at making that change

@KyleKing
Copy link
Owner

KyleKing commented Jun 27, 2023

So this change would break the rendering of one of my test cases

Based on an issue reported to mdformat (executablebooks/mdformat#371 (comment)), the current behavior is necessary for indented paragraphs within a numbered list to be identified. Below is an example:

### With proposed change

1. Here indent width is
   three.

    1. Here indent width is
       three.

1. Here indent width is
   five (three). It needs to be so, because

   Otherwise this next paragraph doesn't belong in the same list item.

---

### With current behavior

1. Here indent width is
    three (four).

    1. Here indent width is
        three (four).

1. Here indent width is
    five (four). It needs to be so, because

    Otherwise this next paragraph doesn't belong in the same list item.

You can see that bullet 2 isn't rendered properly if we were to change the behavior:

Screenshot 2023-06-26 at 9 09 39 PM

@jamesbraza
Copy link
Author

Fwiw, I am not able to repro that screenshot's rendering in PyCharm:

PyCharm rendering

Or in this, this, or this markdown renderer. What renderer are you using?

Fwiw, semantic linefeeds with bulleted lists is pretty fundamental to Markdown. prettier has been around for many years, don't you think this would have been fixed in prettier if it led to rendering issues?

@jamesbraza
Copy link
Author

Looking at the linked issue, I don't know there was ever actually a conclusion, hukkin in his main response disagreed that it was actually a problem. I agree with his comment here:

indent width of ordered lists depends on the width of the initial number of the ordered list

Technically, mdformat-mkdocs breaks this for semantic line feeds right now. For bullets it's not an issue.

What do you think?

@jamesbraza
Copy link
Author

If you don't agree, perhaps mdformat-mkdocs could expose a boolean config option indent_linefeeds defaulting to True to control this behavior

KyleKing added a commit that referenced this issue Jun 27, 2023
@KyleKing
Copy link
Owner

Fwiw, I am not able to repro that screenshot's rendering in PyCharm:

PyCharm rendering

Or in this, this, or this markdown renderer. What renderer are you using?

Fwiw, semantic linefeeds with bulleted lists is pretty fundamental to Markdown. prettier has been around for many years, don't you think this would have been fixed in prettier if it led to rendering issues?

I can reproduce the issue with vanilla mkdocs and created a minimum example here: https://github.com/KyleKing/mdformat-mkdocs/tree/464f4dfe483c602b10425d46770c2b6c4efa338a/mkdocs-demo

Just to understand, are you using this plugin with mkdocs or are you looking for 4-space support in general?

@KyleKing
Copy link
Owner

KyleKing commented Jun 27, 2023

I hacked together a quick working version here: 815e977 and tested both wiht or without the configuration option here: 7fb0476

However, I'm blocked on an approach where the user can specify the opt. Plugins can't currently extend the configuration options (see hukkin/mdformat-web#2 (comment) and https://github.com/executablebooks/mdformat/blob/0cbd2054dedf98ec8366001c8a16eacfa85cebc1/src/mdformat/_conf.py#L63-L69 and executablebooks/mdformat-myst#9 and executablebooks/mdformat#234)

Edit: looks like it should be possible to add CLI options, but not possible to extend the configuration file: executablebooks/mdformat#378 (and there is mdformat-pyproject: https://github.com/csala/mdformat-pyproject/blob/master/mdformat_pyproject/plugin.py)

@KyleKing
Copy link
Owner

KyleKing commented Jun 27, 2023

Published! https://pypi.org/project/mdformat_mkdocs/1.0.2rc2. Try with 'mdformat_mkdocs==1.0.2rc2' however you install mdformat (i.e. pipx inject mdformat 'mdformat_mkdocs[recommended]==1.0.2rc2')

Let me know what you think and if it is behaving properly on your projects

@jamesbraza
Copy link
Author

Oh wow that mdformat's config doesn't yet support extension, that's too bad.

My use case is actually not using MkDocs, it's just vanilla Markdown. Long story short, we have prettier with tabWidth = 4 to match PEP 8 in our JSON, YAML and Markdown files as well. I see mdformat-mkdocs as the way to get mdformat to indent the bullets by 4-spaces (per this issue).


After pip install mdformat-mkdocs==1.0.2rc2 I see:

> mdformat --help
...
  --align-semantic-breaks-in-numbered-lists
                        If specified, align semantic indents in numbered lists to the text

And running, mdformat --align-semantic-breaks-in-numbered-lists README.md it works! 🥳

Thank you! I will use this in pre-commit-config.yaml:

    - repo: https://github.com/executablebooks/mdformat
      rev: 0.7.16
      hooks:
          - id: mdformat
            additional_dependencies:
                - ...  # Other plugins
                - mdformat-mkdocs>=1.0.2rc2
            args: [--align-semantic-breaks-in-numbered-lists]

@KyleKing
Copy link
Owner

KyleKing commented Jun 27, 2023

That is great to hear! I'll go ahead and make the official 1.2.0 (edit: 1.0.2) release then

Let me know if you run into any additional issues

KyleKing added a commit that referenced this issue Jun 28, 2023
@jamesbraza
Copy link
Author

I think that commit has 1.2.0 and 1.0.2, not sure which to pin

@KyleKing
Copy link
Owner

The commit and my comment were incorrect, use 1.0.2!

https://pypi.org/project/mdformat_mkdocs/1.0.2/

@jamesbraza
Copy link
Author

Okay cool. Fwiw, looks like the releases and tags are out of sync somehow too, you might want to fix that

GitHub tags

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

No branches or pull requests

2 participants