gettalong / kramdown

kramdown is a fast, pure Ruby Markdown superset converter, using a strict syntax definition and supporting several common extensions.
http://kramdown.gettalong.org
Other
1.72k stars 275 forks source link

Weird backslash insertion in kramdown writer #751

Closed cabo closed 2 years ago

cabo commented 2 years ago
$ kramdown -o kramdown << HERE
AA: foo

*[AA]: bar
HERE

AA\: foo

*[AA]: bar

I can't find the code that does this backslash insertion. The inserted backslash is stable over further kramdown -o kramdown sequences, but other processors don't like a backslash there (i.e., show it).

gettalong commented 2 years ago

The backslash insert is done here: https://github.com/gettalong/kramdown/blob/master/lib/kramdown/converter/kramdown.rb#L78-L80

I think the code should actually read:

          end.gsub(/\s+/, ' ').gsub(ESCAPED_CHAR_RE) do
            $1 || !opts[:prev] || opts[:prev].type == :br ? "\\#{$1 || $2}" : $&
          end

But have to think a bit more about it.

cabo commented 2 years ago

Ah. I now understand the bug.

$ kramdown -o kramdown <<HERE
a:

*a*:

a*:*
HERE

➔

a:

*a*\:

a*\:*

So the code is trying to protect against spuriously generating definition lists? But ^[ ]{0,3}(:) is matching against the text item, so starting a new text item with a ":" triggers the backslash. Instead, it needs to see whether the text item starting with a ":" lands on a new line on the output.

cabo commented 2 years ago

Couldn't a ":" start a new line in convert_p with :line_width set?

Yep:

kramdown -o kramdown <<HERE | kramdown
aa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa : aa aaaa aaaa aaaa
HERE

➔ 

<dl>
  <dt>aa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa</dt>
  <dd>aa aaaa aaaa aaaa</dd>
</dl>

It's really not that easy to generate markdown...

cabo commented 2 years ago

I'd probably do this two-pass:

  1. generate markdown, but output a special sentinel instead of : for the dt
    • convert all leading : to \: and
    • convert all sentinels to :
gettalong commented 2 years ago

Thanks for the second test case which needed another fix. I will push the commit with the fix later (when it is actually committed and tested ;-)

gettalong commented 2 years ago

@cabo The latest commit contains the fixes for the bugs.

cabo commented 2 years ago

I can report that the fix works for my use case. Thank you!

cabo commented 2 years ago

Now what I need is a release so this bug fix trickles into a few production servers...

gettalong commented 2 years ago

@cabo Will do when I have time, probably on the weekend

gettalong commented 2 years ago

@cabo Release follows shortly