Fix over-zealous backslash replacement

[jcheatham]

Hi,

Two things, the first is to fix the current travis breakage. I wasn't sure why it started with the last build, but it seems like it was broken since the original commit, so maybe some library or gem update? Just looks like the test just needed a \n though.

Second is for #214 (specifically), didn't fix the original emphasis problem in that issue though, looks like that's in char_emphasis, which I'll try and take a look at that when I can. I couldn't find any instances where the backslash needed to be escaped, but please let me know if there's some case I missed (example test would be awesome! :)

And of course please let me know if this needs something else/more to get merged.

Cheers!

[jcheatham]

@robin850 (just in case you didn't get the github ping :)

[robin850]

Hi @jcheatham,

I wasn't sure why it started with the last build, but it seems like it was broken since the original commit, so maybe some library or gem update?

Yep I think it's a Nokogiri update or something like this as the html_equal helper relies on it.

As for the second change, this is pretty strange as the conformance suite doesn't pass on my computer but it does on Travis.

Auto links ... OK
Backslash escapes ... FAILED

2c2
< <p>Backslash: \\</p>
---
> <p>Backslash: \</p>

Blockquotes with code blocks ... OK

I need to investigate a bit before merging this unfortunately. I will also give some feedback a bit later.

Thanks a ton anyway! ❤️

[jcheatham]

No problem, @robin850. Funnily enough this also failed for me at first, but then worked after I got frustrated and did a completely clean build...

The problem is primarily the inconsistency of passing along a single backslash when encountering an unknown escape sequence (e.g. the \p in the test). We could strip the backslash from an unknown sequence or drop the entire sequence altogether... but this seemed like more likely to lead to user confusion, while I couldn't find a strong case for requiring an escape-sequence here.

Happy for any feedback, and merci pour votre attention!

[robin850]

Ok, I'm sorry @jcheatham but I will close this one. Actually we can't remove the back-slash from the list of escaped chars and the markdown you've can't generate the output you're expecting. The original Markdown implementation and Redcarpet both return "\server\path" which seems correct.

This is also the behavior of the grep command:

$ cat foo.txt
\\\\server
\\server
\server

$ grep -n "\\\\server" foo.txt
1:\\\\server
2:\\server
3:\server

Moreover, reading the foo.txt file from Ruby and inspecting it returns a string with 16 backslashes for the first line (which seems pretty big but logical).

Feel free to comment if I'm wrong and I will reopen ; thanks for your work on Redcarpet anyway, we really appreciate ! ❤️

[jcheatham]

Hi @robin850,

Oh, I'm definitely familiar with the madness associated with backslash escape sequence expansion + the shell... I just think in this case it's more a matter of expectation & UX. As a user entering text into a field, even though they might be markdown-savvy they typically aren't expecting to have to deal with using 3 or 4 slashes in order to get 2 when a single slash shows up just fine (when it's not part of an escape sequence) as it's inconsistent. They can (hopefully) click "Preview" on whatever form they're using and get feedback and keep adding slashes as necessary, but it's kind of a poor justification to tell them the reason it has to work that way is "well, it's because a bunch of other tools also exhibit this behavior so we'll just perpetuate it". If the escape sequence is necessary for some other functionality I'm totally oblivious to I'd love to hear it of course, otherwise I think we might just have to fork...

hmmm... or what about if this was done through a run-time or compile-time flag to enable the behavior on demand?

Thanks for the consideration, and I hope you'll give this issue just a bit more from a poor confused user's perspective :)

[robin850]

@jcheatham : At least your points are fair. I will give it some more thoughts ; thanks a lot for your input! :-)

[Stargator]

@jcheatham Can you rebase or resolve the merge conflict?

[jcheatham]

holy necromancy @Stargator , I'll dust off whatever recollection I have and give it my best