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

Redcarpet plugin not handling Guesser::Ambiguous #1343

Closed
johnfairh opened this issue Oct 9, 2019 · 5 comments
Closed

Redcarpet plugin not handling Guesser::Ambiguous #1343

johnfairh opened this issue Oct 9, 2019 · 5 comments
Labels
bugfix-request A request for a bugfix to be developed.

Comments

@johnfairh
Copy link
Contributor

Looking for some advice on how best to fix -- discussion after the form.

Describe the bug
If the Rouge Redcarpet plugin tries to guess the code language and the guess is ambiguous then it propagates an exception back up the stack which abandons the entire document.

To Reproduce

  1. Set up Redcarpet with Rouge plugin.
  2. Pass markdown through Redcarpet:
```
< !DOCTYPE math>
```

(without the space between < and !)

To see this particular ambiguity without redcarpet:

echo '<!DOCTYPE math>' | rougify highlight

Expected behavior
Fall back to either plaintext lexer or one of the ambiguees.

System
Any, though only seen in practice since Mason lexer added in Rouge 3.7.


The redcarpet plugin is very simple -- code is the stuff from within the triple backticks and language is the optional language identifier.

So this just calls Lexer.find_fancy. In the case when no language is supplied by the user we go to Lexer.guess who raises an exception if there is ambiguity.

The particular ambiguity with DOCTYPE has happened because both the xml and mason lexers respond to 'detect' for non-xml/html doctypes.

My first reactive fix is to delete Lexers::Mason.detect? -- I don't think it can be right for this dialect to claim all unclaimed doctypes? But I barely understand what Mason is.

Then a wider question: do users of find_fancy expect it to raise exceptions? It's not documented as such and none of the Rouge-internal users seem to be coded that way. So maybe we should change that interface to return one of the ambiguous lexers. This is a very old public interface though -- probably better to update the docs and update its callers to handle the exception and use one of the lexers therein?

@pyrmont
Copy link
Contributor

pyrmont commented Oct 9, 2019

@miparnisari It seems a mistake to have the Mason lexer match against all documents with a doctype (excepting HTML and XML doctypes). I'm strongly inclined to remove this. Do you remember why it was done this way?

@pyrmont
Copy link
Contributor

pyrmont commented Oct 9, 2019

@johnfairh Mason is a Perl templating language. I'm not sure why it matches against almost all doctypes; that definitely seems like a mistake. @miparnisari did the hard work of adding Mason to Rouge (and I know she uses the lexer) so will check in with her :)

@miparnisari
Copy link
Contributor

@pyrmont hmm no I can't remember. Feel free to remove it,

@johnfairh
Copy link
Contributor Author

Ok, will remove that and add exception handling.

@pyrmont
Copy link
Contributor

pyrmont commented Oct 14, 2019

This has been fixed by #1348 and #1349. Thanks @johnfairh!

@pyrmont pyrmont closed this as completed Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix-request A request for a bugfix to be developed.
Projects
None yet
Development

No branches or pull requests

3 participants