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 ordered / unordered list inconsistent behavior #299

Closed
tiansh opened this issue Nov 2, 2016 · 5 comments
Closed

Nested ordered / unordered list inconsistent behavior #299

tiansh opened this issue Nov 2, 2016 · 5 comments
Assignees
Labels

Comments

@tiansh
Copy link

tiansh commented Nov 2, 2016

INPUT:

* aaa
  1. bbb

PERFER:

  • aaa
    1. bbb
<ul>
  <li>
    aaa
    <ol>
      <li>
        bbb
      </li>
    </ol>
  </li>
</ul>

but got:

<ul>
  <li>
    aaa
  </li>
</ul>
<ol>
  <li>
    bbb
  </li>
</ol>
@tivie tivie self-assigned this Nov 2, 2016
@tivie tivie added the Invalid label Nov 2, 2016
@tivie
Copy link
Member

tivie commented Nov 2, 2016

According to the documentation, nested elements must have, at least, 4 spaces indentation.

* aaa
   1. bbb

You can check the demo with your example working

@tivie tivie closed this as completed Nov 2, 2016
@tiansh
Copy link
Author

tiansh commented Nov 3, 2016

* aaa
  * bbb
* aaa
  1. bbb

But why these two have different behavior?

First one currently been parsed to <ul><li>aaa<ul><li>bbb</li></ul></ul>.
But second one is <ul><li>aaa</li></ul><ol><li>bbb</li></ol>.

@tivie tivie reopened this Nov 3, 2016
@tivie tivie added bug and removed Invalid labels Nov 3, 2016
@tivie
Copy link
Member

tivie commented Nov 3, 2016

You are absolutely right, it's a bug. I will take a look a soon as possible.

Regardless, the 3 space indentation rule should apply to both ordered and unordered lists. They should behave exactly the same.

@tivie tivie changed the title Nested ordered list / unordered list support Nested ordered / unordered list inconsistent behavior Nov 9, 2016
tivie added a commit that referenced this issue Nov 9, 2016
Nested ul and ol lists behave inconsistently in the requirement
of having 3 spaces to be considered a nested list.
This fix changes the requirement to only one space in
both cases.

Closes #299
tivie added a commit that referenced this issue Nov 11, 2016
Acording to the spec, multi paragraph (or block) list item requires subblocks
to be indented 4 spaces (or 1 tab). Although, this is mentioned in the documentation,
Showdown didn't enforce this rule in sublists because other implementations,
such as GFM also didn't. However, in some edge cases, this led to inconsistent behavior,
as shown in issue #299. This commit makes 4 space indentation in sublists
mandatory.

BREAKING CHANGE: syntax for sublists is more restrictive. Before, sublists SHOULD be
indented by 4 spaces, but indenting 2 spaces would work. Now, sublists MUST be
indented 4 spaces or they won't work.

With this input:
```md
* one
  * two
    * three
```

Before (ouput):
```html
<ul>
  <li>one
    <ul>
      <li>two
        <ul><li>three</li></ul>
      <li>
    </ul>
  </li>
<ul>
```

After (output):
```html
<ul>
  <li>one</li>
  <li>two
    <ul><li>three</li></ul>
  </li>
</ul>
```

To migrate either fix source md files or activate the option `disableForced4SpacesIndentedSublists` (coming in v1.5.0):

```md
showdown.setOption('disableForced4SpacesIndentedSublists', true);
```
@tivie
Copy link
Member

tivie commented Nov 11, 2016

should be fixed now in latest development

@tivie tivie closed this as completed Nov 11, 2016
@ellatrix
Copy link

@tivie May I ask why it made more sense to be more restrictive to solve the inconsistency, instead of being less restrictive, and allow a minimum of 2 spaces for mixed lists as well?

This less restrictive syntax doesn't seem to clash with any other syntax. Many people seem to prefer only 2 spaces, which makes sense because it is simpler. Github also allows indentation with 2 spaces.

We are using Showdown in the new WordPress editor, Gutenberg. I tried disableForced4SpacesIndentedSublists, but it doesn't seem to work, and I see it will be removed from v2.

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

No branches or pull requests

3 participants