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
Added support for markdown in heading cells when they are nbconverted. #3576
Conversation
@@ -61,7 +61,7 @@ $$ | |||
|
|||
{% block headingcell scoped %} | |||
|
|||
{{ '#' * cell.level }} {{ cell.source}} | |||
{{ '#' * cell.level }} {{ cell.source | wrap(80)}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does the wrap 80 behave on more than 80 char title ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same way than with other text... it is probably redundant because it is no sense to have a title with more than 80 char at least you are working in the academia... hehe... joke aside, I think we need to wrap the tile for deal with longer ones...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my problem is :
# if this is a really long long title in markdown
this will probably not be...
# if this is a really long
long title in markdown
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you are right about that... maybe is better not to call wrap in this case...
I prefer to have consistency in the title... I will delete this call of the wrap function.
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jinja2 has a built-in wordwrap
filter which support the definition of wrapstrings. If I understand the docs correctly you might be able to apply this filter here, like {{ cell.source | wordwrap(80, False, '\n'+'#' * cell.level)}}
Ok, it is some rude workaround, but should be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, let me try it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could even replace our wrap filter totally I guess !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, it works great... but I added a whitespace to the function to be rendered correctly, if not present we have the text immediately next to #
and it will be not rendered OK by a lot of markdown parsers:
{{ cell.source | wordwrap(80, False, '\n'+'#' * cell.level+' ')}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that it worked!
+2 (+1, +1 because I like nbconverting (verb): IPython conversion process. Example "He was a matlab guy, but he as recently been nbconverted to IPython." |
HeHe... From your definition, "nbconverted to IPython" is redundant... Example "He was a matlab guy, but he as recently been nbconverted." That's all... HeHe As you with iPtyhon vs. IPython campaign... I have to start my own campaign with the |
…p the titles in the correct way.
@@ -61,7 +61,7 @@ $$ | |||
|
|||
{% block headingcell scoped %} | |||
|
|||
{{ '#' * cell.level }} {{ cell.source}} | |||
{{ '#' * cell.level }} {{ cell.source | wordwrap(80, False, '\n'+'#' * cell.level+' ')}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would kill the wordwrap here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and... why? if we have a very long title it would be nice to have it wraped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tools that auto hard-wrap without user input bug me, but you can leave this if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand you, but if we are already wrapping inside markdown cells, I think we have to be consistent and wrap also inside the headers cells... or not wrap anything... If I have to choose, I prefer to wrap it all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consistency argument wins, original comment withdrawn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real question is markdown interpretation is the following one or more title
## Am I a nice
## multiline title or
## 4 titles without
## any text ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(definitively 4 titles if I past that in a MD cell in notebook.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay then, original comment reinstated, do not wrap this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, you convinced me... I will not wrap the headers.
Updating the PR...
…rpretation of multiline headers.
+1 then :-) |
Added support for markdown in heading cells when they are nbconverted.
Thanks! |
Added support for markdown in heading cells when they are nbconverted.
@minrk as posted a PR where provides the possibility to write markdown in heading cells: #3531
@Carreau has pointed out the need of a counterpart in the nbconvert machinery.
@jakobgager and @jdfreder have pointed out that the latex exporter already supported markdown in heading.
So, I have made some little changes for the remaining basic templates: basichtml, rst, markdown (python tpl don't need changes) to cover this issue from the nbconvert machinery point of view.
Damián.