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

Nested lists are too picky about whitespace #368

Closed
chbrown opened this issue Aug 31, 2016 · 12 comments
Closed

Nested lists are too picky about whitespace #368

chbrown opened this issue Aug 31, 2016 · 12 comments
Assignees
Labels

Comments

@chbrown
Copy link

@chbrown chbrown commented Aug 31, 2016

Here's some Markdown markup:

Break.

* Root level
  * Second level
    * Third level
  * Back to second level

Break again.

1. Root level
  * Second level
    * Third level
  * Back to second level

Final break.

1. Root level
   * Second level
     * Third level
   * Back to second level

End of example.

This renders: kramdown output

The middle list should look like the last list.

See this gist for the same input with proper output: https://gist.github.com/chbrown/be1fd39b26da6022846d01d7b2ccfaa4

@gettalong gettalong self-assigned this Sep 1, 2016
@gettalong gettalong added the question label Sep 1, 2016
@gettalong gettalong added the bug label Sep 9, 2016
@gettalong
Copy link
Owner

@gettalong gettalong commented Sep 9, 2016

Thanks for showing this example!

There is actually a bug in kramdown concerning the parsing of the middle list:

1. Root level
  * Second level
    * Third level
  * Back to second level
  1. The first line is parsed, indentation is set to three spaces because the R of Root level is in column three.
  2. The part * Second level is indented two spaces. This means it is part of the first list item because of the "lazy list item content" rule. However, it should not start a nested list because the needed indentation is off (one less than needed). This is the bug.

So the output of the above should actually be:

 <ol>
  <li>Root level
  * Second level
    <ul>
      <li>Third level</li>
      <li>Back to second level</li>
    </ul>
  </li>
</ol>

The part * Third level starts a nested list because its indentation is four and that is more than the needed indentation of three spaces and less than the maximum of six allowed spaces (three for the needed indentation and three for the maximum allowed indentation of a list item marker).

@gettalong gettalong removed the bug label Nov 9, 2016
@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 9, 2016

Just looked at this again but it seems I was mistaken since the latest kramdown version already produces the correct output according to the syntax spec. So there is nothing to do here.

@gettalong gettalong closed this Nov 9, 2016
@chbrown
Copy link
Author

@chbrown chbrown commented Nov 9, 2016

Oh, so, the latest kramdown version renders the middle list like the last one?

Or if not, can you point me to the syntax spec you mentioned?

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 9, 2016

@chbrown It renders as I show in my comment - here is the spec: http://kramdown.gettalong.org/syntax.html#lists

@chbrown
Copy link
Author

@chbrown chbrown commented Nov 9, 2016

Okay. So, to clarify for anyone else who stumbles on this issue:

My desired solution — kramdown not being so nitpicky about indentation — is a no-fix.

Instead, the bug here is that, as of my original post, the version of kramdown that GitHub was using to render Markdown in gists (etc.) was trying to parse the middle-nested list as a list, but it shouldn't have, since it wasn't indented enough. Anything not indented 'enough' should, according to the spec @gettalong linked to, be interpreted literally.

I'm still not clear why the "Back to second level" item stays in the same list as the "Third level" item, but the parser has already diverged by that point from what I was intending with the three levels, so perhaps whatever it's trying to do there is outside the scope of needing rationalization.

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 9, 2016

kramdown is not "nitpicky", it just tries to follow a sensible specification. Try using other Markdown parsers and you will get many different behaviours, with many edge cases and cases where you can't tell from the source code what the output will be.

As for Github gists: They use a different Markdown dialect, not kramdown, for gists, so yes, the behaviour will differ in many cases. There is no Markdown spec.

As for the problem at hand, I have to apologize since it seems that some experimental code came into play when I tested - so the problem still needs to be fixed for the next release. I'm sorry for mixup!

@gettalong gettalong reopened this Nov 9, 2016
@gettalong gettalong added the bug label Nov 9, 2016
@chbrown
Copy link
Author

@chbrown chbrown commented Nov 10, 2016

Github gists: They use a different Markdown dialect, not kramdown

You're right; a lot (😱) has happened since Aug 31 and I forgot that the gist renderer had the flexibility I wanted — the problem was that GitHub pages' Jekyll config demands using kramdown, and I'm rather fond of GitHub pages and Markdown. (Jekyll not so much, but sometimes it's worth taking a bit of Ruby medicine to avoid having to keep rendered files in source control.)

There is no Markdown spec.

Which is why I can get away with arbitrary prescriptive judgments like 'nitpicky' 😉

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 10, 2016

I have looked at the issue in more detail now and there was another problem with the behaviour of the "Back to second level" item. So kramdown will now output the following:

<ol>
  <li>Root level
  * Second level
    <ul>
      <li>Third level
      * Back to second level</li>
    </ul>
  </li>
</ol>

Note that the output will be different when using the GFM parser if the option "gfm_quirks" includes "paragraph_end" which is the default!

@chbrown
Copy link
Author

@chbrown chbrown commented Nov 10, 2016

Excellent, that output seems properly and entirely consistent. It's not the policy I'd like, but it makes sense, and you've got to draw the line somewhere. I'm happy for you to close this issue at this point.

@IanLee1521
Copy link

@IanLee1521 IanLee1521 commented Nov 11, 2016

Hi there! I was emailing with GitHub support yesterday about some list differences I was seeing:

there is some other strange list differences that I’ve seen between the GitHub.com http://github.com/ in browser renderer and GitHub Pages:

GitHub Pages (http://software.llnl.gov/about/using-github/ http://software.llnl.gov/about/using-github/) messes up the numbered list continuation and sub-bullet lists which GitHub.com http://github.com/ handles correctly / as I expect (https://github.com/LLNL/llnl.github.io/blob/master/about/using-github.md).

And they pointed me to the following being the reason for the difference:

The github.com engine recently got updated to bring it into line with the commonmark specification, but kramdown isn't yet compliant with that, hence the difference

Which after going searching and finding: http://spec.commonmark.org seems counter to the comment above:

The is no Markdown spec

Is moving in the direction of that spec anything that you / the Kramdown team would be interested in?

Thanks!

@gettalong
Copy link
Owner

@gettalong gettalong commented Nov 11, 2016

@IanLee1521 As I said, there really is no Markdown spec. I could also point you to the kramdown specification at http://kramdown.gettalong.org/syntax.html and ask you to change the Markdown parsers so that they comply with this spec.

The only advantage CommonMark has is that it has the word "Common" in its name, leading to the believe that it is a standard. It certainly may become one but currently, as I see it, it is not.

Also, I haven't yet seen any advantage of the CommonMark format in comparison to the kramdown format but I have to admit that I haven't checked for a while.

And a last point: There is no need for kramdown to move into the CommonMark direction. If you want to use CommonMark, just use a CommonMark parser 😄

@gettalong gettalong closed this Nov 12, 2016
gettalong added a commit that referenced this issue Nov 13, 2016
@IanLee1521
Copy link

@IanLee1521 IanLee1521 commented Nov 14, 2016

Got it. Thanks @gettalong !

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.