Add an option to render to span elements #10
Conversation
| def verify(self, mkd_name, html_name, enable_dollar_delimiter=False): | ||
| config = {'enable_dollar_delimiter': enable_dollar_delimiter} | ||
| def verify(self, mkd_name, html_name, enable_dollar_delimiter=False, | ||
| render_to_span=False): |
There was a problem hiding this comment.
What's the point of adding a new argument if you don't use it?
There was a problem hiding this comment.
Just for consistency with enable_dollar_delimiter which was already there. Should render_to_span be used differently than enable_dollar_delimiter then?
|
Thanks for your pull request and sorry for the delay (I somehow marked the mail as read and forgot about it). I have now posted some comments. If you amend the commit with fixes for them, I will merge it. |
| config = {'enable_dollar_delimiter': enable_dollar_delimiter} | ||
| def verify(self, mkd_name, html_name, enable_dollar_delimiter=False, | ||
| render_to_span=False): | ||
| config = {'enable_dollar_delimiter': enable_dollar_delimiter, |
There was a problem hiding this comment.
enable_dollar_delimiter is actually used in one of the tests. I don't mind if you test your option too, but if you don't, please remove the useless argument.
There was a problem hiding this comment.
I'd rather not add a test. I thought it more logical to add an argument even if currently unused, in case some test for it were added later. But OK, I'll remove it.
a3nm
force-pushed
the
master
branch
2 times, most recently
from
af04cf8 to
5e0cf90
Compare
|
On Wed, Feb 10, 2016 at 11:11:26AM -0800, Dmitry Shachnev wrote:
No worries, thanks for the review! Should be OK now, please have a look. Best, Antoine Amarilli |
|
Thanks! I will merge it tomorrow morning. |
I wanted to render MathJax to span elements so that it also works on browsers without JavaScript support or with JavaScript disabled, so I added an option to do this. Do you think it is reasonable to merge? Thanks!