jupyterlab / jupyterlab-toc

Table of Contents extension for JupyterLab
BSD 3-Clause "New" or "Revised" License
727 stars 107 forks source link

Additional markdown headings in cell are ignored #54

Open kenjioman opened 5 years ago

kenjioman commented 5 years ago

Thanks again for the continued progress! I've noticed a regression -- if a markdown cell in a notebook has more than one heading, for example:

# Level 1 heading
## Level 2 heading
# New level 1

Only the first Level 1 heading shows up in the TOC. It used to be the case that all headings were there, but now, to get them to show up, I have to split these markdown cells (into 3, for this example) so each has the heading as the first line of the cell. It would be nice if we were able to have the previous behavior back where all headings within a cell are captured.

ian-r-rose commented 5 years ago

This change was actually intentional (though I am not sure that it is the right choice :smile:). We would like to have new collapsing/expanding behavior also be allowed to apply to hiding/showing of cells (n.b., this is not implemented yet). Such behavior becomes very difficult to reason about once there are multiple headings in a single cell.

What are your thoughts on the issue? I was a bit uncomfortable about the change at the time...

kenjioman commented 5 years ago

I see your reasoning/ the direction you're going. I guess if I know that other headings within a cell will not be captured by TOC, it's not too big of an issue to make sure I split them -- the biggest issue for me now is that I have notebooks from before this change where it is currently harder to find some of my sections because the relevant headings are missing from TOC (but with time, I can switch to this new organization method).

One idea, if you want to capture every heading in TOC, but still allow for hiding specific cells -- perhaps don't allow hiding any cells in the notebook with more than one heading, and have documented that if one wants to collapse parts of these cells, users will have to split them (recovering the current behavior).

Also, how do you intend to handle hiding cells that have multiple headings in their output (as we talked about in #15?) Assuming you don't want to try to handle only showing some of the output based on what TOC entry is collapsed, I assume it's easiest if the first of such headings indicates it can be hidden, with none of the rest having that indicator. Then, toggling this first heading indicator will hide/show all remaining output with it. Does that sound reasonable?

msiemens commented 5 years ago

I'm also interested in this, I have notebooks with a lot of markdown in a single markdown cell which currently only generates a single TOC entry, which makes it kind of useless 🙈

ian-r-rose commented 5 years ago

What do you think about this @markellekelly? Should we restore the rendering of all markdown cells? I wasn't around for some of the design discussions you all had, so some additional insight would be useful...

markellekelly commented 5 years ago

@ian-r-rose I'm definitely open to including all markdown headings, but we should have a discussion of what exactly that would look like in the context of hiding cells. I think hiding the first header (or any header) of a markdown cell could make sense for hiding the entire cell - @ellisonbg might have some thoughts on this as well.

sebkopf commented 5 years ago

Love the new TOC features but it would indeed be great to include all markdown headings in a single cell in the TOC again, even if that means they cannot be collapsed. Specifically running into this problem in use cases where notebooks are automatically generated (e.g. from RMarkdown files or other plain text / non JSON formats) without clear delineation of cells so all continuous markdown is usually converted into a single cell.

diallobakary4 commented 5 years ago

It will definitely be more helpful to include all headings in a cell, just noticed the behaviour after pasting a long text.

spencermathews commented 5 years ago

I definitely appreciate that including all headers in a cell could lead to some complicated interactions with potential future features. Regardless of how that might be resolved, I would submit that including all headings in the TOC is simply the expected behavior, regardless of if they happen to be in the same cell.

stephenkraemer commented 5 years ago

If you consider re-enabling display of all headings, please also consider enabling it in programmatically generated HTML and Markdown (though that may be an automatic consequence of re-enabling this feature?)

I think there are a lot of use cases where displaying all programmatically generated headers is of great value! For example I often have to screen parameter combinations, for which I create one plot per combination. Creating a hierarchy of headings which I can browse with the TOC has been a great help in these situations. I realize that this is not trivial, but it would be awesome if you could enable this again! I think the ability to programmatically generate reports within the notebook really sets this environment apart from other alternatives.

john-kang commented 4 years ago

Would it be possible to have an option to toggle this on/off?

kgryte commented 4 years ago

The reality is that incorporating additional Markdown headings is hard to implement within the context of other ToC features, such as collapsing notebook sections, etc.

As mentioned above, but just to reiterate, once we incorporated support for collapsing notebook sections within a notebook and not just in the ToC, the presence of multiple headings in a single cell becomes very difficult to reason about, as this entails (a) that we'd impose a separate document structure independent of the underlying notebook structure (i.e., cells no longer serve as natural section boundaries), thus requiring a separate document model and model coordination and (b) that we'd have to manipulate cell contents (in a much more invasive way) in order to incorporate UI elements which we get for essentially free from the underlying notebook model. This is especially true for cells which output Markdown and HTML.

In short, from the perspective of a collapsible ToC, multiple headings within a single cell would impose a significant development burden and significantly increase extension complexity.

I think the reality is that, while it would be nice to support all features and possible use cases, this extension simply cannot. For particular use cases, such as report generation, other possibilities exist that allow you to still take advantage of the ToC extension. For example, instead of having a single notebook which both generates a report and includes that very report, one can programmatically generate a separate report notebook which can then be programmatically executed. (note that this also has the advantage of being able to create multiple independent artifacts for checkpointing, etc.)

In general, my recommendation is to follow a one heading-defined section per cell model, as such a model makes it considerably easier to reason about a notebook document.

ellisonbg commented 4 years ago

I agree with only handling a single markdown heading at the top of a cell for the purpose of the TOC.

On Tue, Nov 12, 2019 at 2:55 PM Athan notifications@github.com wrote:

The reality is that incorporating additional Markdown headings is hard to implement within the context of other ToC features, such as collapsing notebook sections, etc.

As mentioned above, but just to reiterate, once we incorporated support for collapsing notebook sections within a notebook and not just in the ToC, the presence of multiple headings in a single cell becomes very difficult to reason about, as this entails (a) that we'd impose a separate document structure independent of the underlying notebook structure (i.e., cells no longer serve as natural section boundaries), thus requiring a separate document model and model coordination and (b) that we'd have to manipulate cell contents (in a much more invasive way) in order to incorporate UI elements which we get for essentially free from the underlying notebook model. This is especially true for cells which output Markdown and HTML.

In short, from the perspective of a collapsible ToC, multiple headings within a single cell would impose a significant development burden and significantly increase extension complexity.

I think the reality is that, while it would be nice to support all features and possible use cases, this extension simply cannot. For particular use cases, such as report generation, other possibilities exist that allow you to still take advantage of the ToC extension. For example, instead of having a single notebook which both generates a report and includes that very report, one can programmatically generate https://github.com/maxalbert/auto-exec-notebook/blob/master/how-to-programmatically-generate-and-execute-an-ipython-notebook.ipynb a separate report notebook which can then be programmatically executed. (note that this also has the advantage of being able to create multiple independent artifacts for checkpointing, etc.)

In general, my recommendation is to follow a one heading-defined section per cell model, as such a model makes it considerably easier to reason about a notebook document.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jupyterlab/jupyterlab-toc/issues/54?email_source=notifications&email_token=AAAGXUA67BZRTDYNRZGJVQTQTMX65A5CNFSM4FXGBKK2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOED4HWKQ#issuecomment-553155370, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAGXUEOLMDQFGJKCWLMMOLQTMX65ANCNFSM4FXGBKKQ .

-- Brian E. Granger

Principal Technical Program Manager, AWS AI Platform (brgrange@amazon.com) On Leave - Professor of Physics and Data Science, Cal Poly @ellisonbg on GitHub

kenjioman commented 4 years ago

I agree with the toggle feature.

A naive/ new user, I believe, will expect the table of contents to capture the full structure (headings/ sub-headings) of a document. Granted, going forward, document creators, knowing the limitation of jupyterlab's TOC, can design their document with the constraint of having headings/sub-headings broken out into their own cells. However, the naive assumption will fail for past notebooks that weren't designed this way.

I also recognize the desire for the collapse state from the TOC to propagate to what of the notebook is actually displayed as well (and the complexities this brings in the document model that kgryte mentioned above in https://github.com/jupyterlab/jupyterlab-toc/issues/54#issuecomment-553155370). In order to get the best of both worlds, I think a toggle would make sense -- toggled one way, all headings show up, with collapse-ability turned off. Toggled the other way, notebooks sections can be hidden by TOC, but any additional headings don't show up in TOC.

Granted, as a user that is not able to contribute to this feature request, I am fully appreciative of what has been done, what will be done, and design decisions made to simplify development :)

Thanks again for all the awesome work with jupyterlab and TOC! This is a great contribution to the world: 🌎 🌏 🌍 😄

kenjioman commented 4 years ago

One more thing I should probably point out -- personally, I don't think I care to have/ need sections of the document to show/be hidden based on TOC collapse state. However, I do appreciate the ability to have sections in the TOC collapse/fold, letting me hide away sections of the TOC that I'm not interested in at the moment.

So, I guess another sub-feature request as a part of the toggle -- decouple the TOC collapse state from the actual document state. So, in summary, I see these two toggle states:

I hope that was clear! Also, I think some of the TOC folding is already implemented, but I'm not in front of jupyterlab at the moment, so I can't confirm ...

augustosisa commented 3 years ago

Something about this issue? I will appreciate the ability to display ALL headings in a cell too as in the classic Jupyter Notebook. Maybe as an option if there are features where those texts will be annoying.