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

allow markdown in heading cells #3531

Merged
merged 1 commit into from Jul 9, 2013
Merged

Conversation

minrk
Copy link
Member

@minrk minrk commented Jul 4, 2013

italics, bold, math, etc.

closes #3053

italics, bold, math, etc.

closes ipython#3053
@Carreau
Copy link
Member

Carreau commented Jul 5, 2013

This should probably get its counterpart in nbconvert as the same time.

@jakobgager
Copy link
Contributor

For latex conversion this is already in nbconvert IIRC! We used this to take care of the underscore escaping, cf ipython/nbconvert#171

@jdfreder
Copy link
Member

jdfreder commented Jul 5, 2013

I think @jakobgager is right about the latex, at least the sphinx templates support this. The other templates do not.

@damianavila
Copy link
Member

I will try to work in the others template to support this... we need markdown in header cells.

Carreau added a commit that referenced this pull request Jul 9, 2013
allow markdown in heading cells
@Carreau Carreau merged commit 8206b14 into ipython:master Jul 9, 2013
@damianavila
Copy link
Member

Great! Thanks.

@jakobgager
Copy link
Contributor

There seems to be a bug with markdown in heading cells in the current master (pulled 5 minutes ago).
If markdown (italics, math, etc) is added to the header the cell (and all other) cannot be executed anymore.
At this stage Shift+Return adds a new line in all present cells no matter if code or markdown.

Might add: Chrome 28.0.1500.71 on Ubuntu 12.04

@damianavila
Copy link
Member

I can confirm this behavior, but only with math (italics, bullet points and so on, seems to work right for me).
I tested in Cromium and FIrefox on Ubuntu 12.04.

@jakobgager
Copy link
Contributor

For me it freezes even if I do something like
Foo *bar*
Bullets do not freeze the notebook but are ignored - however I guess this intended!
The funny thing is, once the markdown is erased form the heading cell everything works fine again ❓

Update: I get the same behavior with FF 22.0

@Carreau
Copy link
Member

Carreau commented Jul 10, 2013

Big problem in mathjax math handeling. Marking as Prio Blocker 1.0 will revert markdown in heading cell temporarly

@Carreau
Copy link
Member

Carreau commented Jul 10, 2013

Be carefull not to leave any markdown in cell, it might prevent to load.

@Carreau
Copy link
Member

Carreau commented Jul 10, 2013

There is multiple issues,

  1. rendering html in header cell was done manually and was expecting text. Parsing markdown return dom-object, that if simple enough (only 1 node) behave as strings.
  2. mathjax use a module local to retain math value, which if you fix some other things enter in a race condition.
    Not sure why this one we don't see with only MD cells...

@Carreau
Copy link
Member

Carreau commented Jul 10, 2013

Reverted in #3595 will open an issue to re-do this PR.

@jakobgager
Copy link
Contributor

@Carreau you are extremely fast 👏

@ellisonbg
Copy link
Member

Hold on I thought we were talking about not doing full markdown in heading cells. Did that get merged?

Sent from my iPhone

On Jul 10, 2013, at 7:14 AM, Jakob Gager notifications@github.com wrote:

@Carreau you are extremely fast


Reply to this email directly or view it on GitHub.

@Carreau
Copy link
Member

Carreau commented Jul 10, 2013

Hold on I thought we were talking about not doing full markdown in heading cells.
Did that get merged?

Yes, to have at least emphase and bold. So this was not full markdown support... even if you could have put anything in it.

BTW, after reverting I can stil-already have math in heading cell.

@damianavila
Copy link
Member

BTW, after reverting I can stil-already have math in heading cell.

Me too... do you know why?

@ellisonbg
Copy link
Member

I am still confused because that is not the direction things were headed in
last time I checked. I am still -1 on heading cell supported anything
other than inline math without further discussion about the consequences of
adding markdown support. IOW, I don't think heading cells should support
emphasis and bold for 1.0.

On Wed, Jul 10, 2013 at 9:01 AM, Matthias Bussonnier <
notifications@github.com> wrote:

Hold on I thought we were talking about not doing full markdown in heading
cells.

Did that get merged?

Yes, to have at least emphase and bold. So this was not full markdown
support... even if you could have put anything in it.

BTW, after reverting I can stil-already have math in heading cell.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3531#issuecomment-20752793
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@jakobgager
Copy link
Contributor

Math, but not markdown, in heading cells was supported since at least 0.13.
So I guess the "old" state was fine than?!

@ellisonbg
Copy link
Member

I think so...

On Wed, Jul 10, 2013 at 9:55 AM, Jakob Gager notifications@github.comwrote:

Math, but not markdown, in heading cells was supported since at least 0.13.
So I guess the "old" state was fine than?!


Reply to this email directly or view it on GitHubhttps://github.com//pull/3531#issuecomment-20756538
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@ellisonbg
Copy link
Member

Let me put my thinking in a different way: changing heading cells from raw text to markdown constitutes a major change in the notebook format that we should not be doing without more consideration and incrementing the notebook format.

@minrk
Copy link
Member Author

minrk commented Jul 10, 2013

@ellisonbg totally disagree. Heading Cell is just a text cell with one tiny piece of implicit formatting and semantic structural meaning. That has no further implications. I have always considered it a bug that the input is not rendered, and there is nothing lost by fixing this bug.

@ellisonbg
Copy link
Member

I don't see how this is not a format change. Before the change, heading
cells needed no rendering in the UI or nbconvert or anyone elses toolchains
(emacs notebook). After the change everyone has to render heading cells as
markdown. I agree it doesn't break existing notebooks, but it is still a
change to the format.

Because it was a deliberate choice to not allow markdown in heading cell, I
don't consider this a bug in the purest sense. I do agree that some users
are surprised though that markdown doesn't work in heading cells - that
does have a buggy feel.

Downsides of this change:

  • With Markdown allowed in heading cells, people will start to enter other
    markdown (including # based headings) in heading cells. Our situation with
    multiple ways of entering headings is already confusing enough - this makes
    it worse.
  • It forces other parts of our toolchain as well as other projects (emacs
    notebook) to update their handling of markdown.
  • If we ever want to move away from Markdown for heading cells (if we
    create out own mutant love child of markdown and rest) it would be
    difficult to transition heading cells to that love child. OTOH, if that
    happens we would not remove markdown cells - we would add a new type of
    cell for our new markup syntax. A perfect example of where this would come
    up is in allowing rest/latex style labeling of heading cells. If we want
    to allow for that, it probably won't be markdown.

I would like to have a better sense of what our long term markup syntax
plan is before committing to this change. If we are 100% positive that
heading cells will never be anything but markdown, I am open to this change
going through now. But based on our recent discussion, I am more convinced
than ever that we won't be markdown centric forever.

On Wed, Jul 10, 2013 at 10:02 AM, Min RK notifications@github.com wrote:

@ellisonbg https://github.com/ellisonbg totally disagree. Heading Cell
is just a text cell with one tiny piece of implicit formatting and semantic
structural meaning. That has no further implications. I have always
considered it a bug that the input is not rendered, and there is nothing
lost by fixing this bug.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3531#issuecomment-20757011
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@minrk
Copy link
Member Author

minrk commented Jul 10, 2013

  • With Markdown allowed in heading cells, people will start to enter other
    markdown (including # based headings) in heading cells. Our situation with
    multiple ways of entering headings is already confusing enough - this makes
    it worse.

The part about # is not actually true - the formatting should be perfectly identical to having already typed # The heading. PR #3601 ensures this by actually rendering the html inside a header tag, rather than my first draft which rendered the inner HTML as a standalone blob.

  • It forces other parts of our toolchain as well as other projects (emacs
    notebook) to update their handling of markdown.

Only in that they should treat heading cells with the following logic:

  1. prepend '#' * level + ' '
  2. treat exactly the same as markdown cell

This is actually how nbconvert used to work (I don't know how many times it's been rewritten since then, so it may not now).

  • If we ever want to move away from Markdown for heading cells (if we
    create out own mutant love child of markdown and rest) it would be
    difficult to transition heading cells to that love child.

This is true - if we ever want to replace markdown in heading cells with something new that broke backward compatibility, we would need a new nbformat. I'm not sure "makes it harder to create a mutant love child" is a particularly strong argument, though.

Here's what the heading cell has always meant to me:

  • adding semantic / structural information to regular headings

It has always been a small, easily fixed, bug that we forgot to properly render the text in a heading cell.

I cannot imagine that we would want to change that, and the lack of formatting means that users have to choose between formatting and structure, which is a clear failure of design.

@jakobgager
Copy link
Contributor

I totally agree with @minrk that, as long as headings are style-able if entered as markdown cell (using #) there should be an option to style the heading cells as well, just for consistency reasons.
IMO there is no need to format headings as their format should be exclusively controlled by the CSS or Latex style anyway, however math is essential in headings. But that is only my personal opinion.

@damianavila
Copy link
Member

Me too... as a user, my notebooks don't have heading cells, because they can be formatted...
And regarding what @minrk has said...

Only in that they should treat heading cells with the following logic:

prepend '#' * level + ' '
treat exactly the same as markdown cell

it was very simple to accommodate nbconvert to render markdown in heding cells instead of raw text, example here: #3576.

@jakobgager jakobgager mentioned this pull request Jul 10, 2013
7 tasks
@ellisonbg
Copy link
Member

I don't agree with this perspective, but I am fine going with it.

@jakobgager
Copy link
Contributor

Sorry @damianavila but I could not follow your preference. I personally think there is no need to allow markdown in heading cells, but allow mathjax math rendering. That is, I would stick with the current 0.13 implementation.

Unfortunately, this is somehow inconsistent with the headings which can be created based on simple markdown cells.

@damianavila
Copy link
Member

I agree with @minrk here, I think it is a failure of design the fact that you have to choose between formatting and structure...
If I want to structure my document using heading cells (which is great thing to have) then I would need these headers formatted as the main text of my document (essentially markdown cells for text)... but at the end, it seems a question of personal preference.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
allow markdown in heading cells
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Markdown in header cells is not rendered
6 participants