Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Nested pre formatting #165

Open
swashcap opened this issue Mar 2, 2016 · 24 comments
Open

Nested pre formatting #165

swashcap opened this issue Mar 2, 2016 · 24 comments

Comments

@swashcap
Copy link

swashcap commented Mar 2, 2016

Hello! I just published hawkify-pouchdb. I noticed that a code block didn’t look right:

code-block

After exploring the DOM’s order, it appears the Markdown parser is moving the pre outside of the ol:

source-order

However, if you check out the Markdown source, the code block is clearly nested:

markdown-source

Any thoughts?

@wooorm
Copy link

wooorm commented Mar 2, 2016

That happens because 2.· (middle dot represents space) is three characters, and the code-block is indented with two characters.
GitHub and the original Markdown allow that, but the CommonMark spec (source), which markdown-it (which is used by this project) follows, does not!
To fix this for now on your side, you could add an extra space before the fenced code-block and its following paragraph.

@revin
Copy link
Collaborator

revin commented Mar 2, 2016

Thanks @wooorm I was looking for that in the spec. 👍

@ashleygwilliams GitHub renders this as @swashcap intended; should we look into relaxing the indentation requirement?

@revin
Copy link
Collaborator

revin commented Mar 2, 2016

  1. Test 1
  2. Unindented code block follows:
  {
    foo: 'bar'
  }
  1. Test 2

  2. Indented one space

     {
       foo: 'bar'
    }
  3. Test 3

  4. Indented 2 spaces

      {
        foo: 'bar'
      }
  5. Test 4

  6. Indented 3 spaces

     {
       foo: 'bar'
     }

So it looks like a single space indent puts the code block inside a list item, but the code itself is visually misaligned; anything more than one space properly nests.

@revin
Copy link
Collaborator

revin commented Mar 2, 2016

More testing. Here's a screenshot of some markdown:


screen shot 2016-03-02 at mar 2 12 40 56 pm


Here's GitHub's rendering:

For that matter, let's test simple list item continuations.

  • first item
  • second item
    no indent
  • another item
  • additional item
    one space indent
  • continuing item
  • further item
    two space indent

Here's how markdown-it renders it:

screen shot 2016-03-02 at mar 2 12 39 10 pm


Sigh.

@ashleygwilliams
Copy link
Contributor

hey @revin -- this is a tricky sitch, but i think that parity with GitHub is still our product roadmap. it's a little gross, so perhaps we should create a layer of transformations that are single files that we keep in a directory for github specific tweaks so we can remember that we think it is a little gross and make it em easy to remove if GitHub ever changes?

@revin
Copy link
Collaborator

revin commented Mar 3, 2016

@ashleygwilliams Do you think it's worth having an option that will toggle marky between CommonMark & GFM modes?

@ashleygwilliams
Copy link
Contributor

Perhaps in the long run @revin ? I always tend to prefer convention over config though. When i think of "what is marky" the answer is: a node.js verison of the GH markdown parser, so that toggle would be out of scope. I'm open to it, of course, but i don't think it is a priority.

@revin revin removed the question label Mar 3, 2016
@revin
Copy link
Collaborator

revin commented Mar 3, 2016

OK, works for me. Your "what is marky" definition helps; thanks for clarifying 👍

@revin
Copy link
Collaborator

revin commented Mar 11, 2016

Hi all, quick status update: this has led me down quite a rabbit hole trying to replicate GitHub's behavior surrounding the interactions between lists and code blocks. I've made quite a bit of progress, but there's still a little bit more to do, plus write tests for all of it; hopefully I'll have something to look at next week.

@revin
Copy link
Collaborator

revin commented Mar 22, 2016

sigh ok, another update. I've got a whole suite of markdown-it plugins I've written to handle this and other differences with GitHub's list processing code. I've written it and junked it and rewritten it, and even though the second time through is a big improvement on the first, the modifications are still bigger in terms of code than if I'd just forked markdown-it's ilst parsing rule and replaced it with a modified one. So I'm going to try that next; it should be fewer/faster lines of code in the end, but it does mean moar waiting. Sorry about that.

@ashleygwilliams
Copy link
Contributor

update on this @revin? if you aren't still working on it, i might add PRs welcome - thoughts?

@revin
Copy link
Collaborator

revin commented Oct 14, 2016

eek! 😳 I'm so sorry @ashleygwilliams I forgot to update this; I haven't yet been able to make a real effort at doing the markdown-it list parsing rule change I mentioned. I'm nervous about trying to maintain something like that, but I think in the end it's the simplest way 😕 . I'll add the label for the time being.

@jamestautges
Copy link
Contributor

@revin Would you mind if I took a crack at this? This sounds a lot like what I did with the html headings.

@revin
Copy link
Collaborator

revin commented Oct 26, 2016

@jamestautges absolutely, go for it. As I said above, it's basically a matter of forking markdown-it's list parsing rule and replacing it completely with one that works the way github's parsing does (which is pretty flakey, to be honest). Plus tests, etc...

@ashleygwilliams
Copy link
Contributor

PRs are super welcome @jamestautges we'd love if this got fixed :)

@jamestautges
Copy link
Contributor

Well, by playing around I've learned some slightly horrifying things about how github parses its lists. That being said, I think I can write a parser in the next couple of days.

  1. The weird spacing stuff you were showing earlier has more to do with whether the list item is at the end of the block than the number of spaces
  2. You can mix ordered and unordered list items and it only depends on the first one
    • You would think that increased levels would be caused by increased indentation
      • But that's not always true
  • item 1
  • item 2
    append!

Break the block

  • item 1
  • item 2
    Add to item???
  • item 3

@revin
Copy link
Collaborator

revin commented Oct 27, 2016

Yep it's not even close to how CommonMark specifies things, and often depends on the relative difference between lines and unexpected things like that.

It may be instructive to check out https://github.com/github/markup for some insight into how github's stuff works. In particular the markdown library I think is at work here is https://github.com/github/markup IIRC, the main guts of which are written in C. I'd been planning on analyzing that and porting over the basic list algorithm but haven't been able to spend much time on it yet.

@jamestautges
Copy link
Contributor

Great. Just discovered that gists and the preview here parse lists differently 😭

@revin
Copy link
Collaborator

revin commented Oct 27, 2016

Yeah, I've sometimes taken to pushing forks/branches of projects just to see what the real rendered readmes look like for sure. Very tedious ☹️

@jamestautges
Copy link
Contributor

Am I right in thinking that the original use case here isn't possible in github's readme parsing? No matter how much indentation I use, I can only get it inline or entirely out of the list.

@revin
Copy link
Collaborator

revin commented Oct 27, 2016

Not sure if I understand your question correctly, but the original readme at https://github.com/MRN-Code/hawkify-pouchdb has a nested/indented code block. Looks like the blank line between the list item text and the code block is required, or else it inlines the code block.

I mean basically the real issue at stake here is GitHub's algorithm for deciding how much indentation maps to what list nesting level is just very unconventional. 😕

@daphee
Copy link

daphee commented Oct 30, 2017

I stumbled across this issue and while digging through the comments I discovered that in the original example GH seems to now follow the CommonMark spec in that it breaks out the <pre> out of the <ol> as well.

I couldn't follow the other "hidden rules" of GH list nesting you guys found but maybe those need to be re-evaluated as well and maybe this issue and the PR can be closed.

@swashcap
Copy link
Author

Silly Github:

gosh-dangit-github

Yeah, they totally changed their whitespace indentation detection. 😱

@revin @jamestautges @ashleygwilliams I’m fine with closing if you are. No sense in trying to align to an old, undocumented Markdown parser.

@revin
Copy link
Collaborator

revin commented Nov 3, 2017

@swashcap yeah I suspect this will end up becoming obsolete once we get #394 merged, since GH is now more of a superset of CommonMark rather than a custom markdown implementation. We'll go through and close things like this out then. Thanks!

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

No branches or pull requests

6 participants