jupyterlab/frontends-team-compass

Merge JupyterLab-TOC to JupyterLab core

Closed this issue · 12 comments

As suggested in the TOC repo and discussed on the JupyterLab dev meeting on May 20, we will start the process to move TOC to JupyterLab core.

yessssss

:shipit: 💯 👍

I support moving TOC to core.

I also support moving it to core. I suggested in today's meeting that we bring it in as-is, and then make it not visible by default (to preserve space in the side bar), but easily enabled using a menu item that is backed by a setting.

Very cool! A few thoughts about the process of migrating into core, and why I have been a bit reticent about it in the past:

  1. There is currently registry pattern somewhat loosely modeled after the rendermime registry. We should make sure that this is exposed to extension authors with the ITableOfContentsRegistry token and the JLab standard @jupyterlab/toc/@jupyterlab/toc-extension division of responsibilities.
  2. There is some awkwardness around the API design for the ToC toolbar. There is no documentation about how to use it, and I'm not sure that there is anybody around who could easily generate it (I don't really understand it at the moment). They deserve to be rethought and cleaned up, whether before or after migration.
  3. The logic around hiding and collapsing notebook cells involves some DOM manipulation of things that don't belong to the extension (that is, notebook cells). This has proven confusing to users. I think there should be some unification with the native JupyterLab collapsing UX and cell metadata, but others have disagreed with me on this (cc @kgryte).
  4. Similarly, the auto-numbering functionality does some DOM manipulation that makes me uncomfortable. We might want to remove that.
  5. The current ToC extension blesses LaTeX documents as one of the doctypes which it knows how to generate a ToC for. Do we want to continue that?
  6. The HTML, Markdown, and LaTeX ToC generators currently have some hand-written best-effort regexes for identifying headers. Obviously, I should never have done this, but here we are! Do we want to revisit that?
  7. The ToC itself could probably use a design/CSS pass (:wave: @tgeorgeux )
  8. The current extension has functionality for previewing markdown and code cells in the ToC itself. While a cool bit of hacking, I kind of feel that it is a bit antithetical to the purpose of a table of contents. Should we retain that?

As discussed previously (i.e., at the JLab dev sprint), core needs to provide various APIs for collapsing groups of cells. But this was left unresolved.

Not clear to me that ToC should be merged as is. This would be just moving the headache around. The extension is a nice PoC, but I'd advise taking a step back and asking the question: "if we did this in core today, from scratch, how would we implement it and what changes would we need to make to make that happen?" Once answered, can compare to the extension implementation. I am not convinced those two visions will be the same.

As is, the extension basically has to work around limitations in core. These have been detailed and discussed previously on issue threads and in PRs.

I think a larger question for me is: what about the ToC extension makes it an essential part of the lab experience? Yes, the extension is relatively popular, but I would not consider essential. So not clear to me what the criteria is for pulling some things into core and not others. For example, if popularity is the metric, then the Git extension would seem like a prime candidate, but this does not seem "right".

Whether or not ToC is in core, APIs need to be exposed to overcome the current ToC workarounds, particularly in facilitating bidirectional ToC/notebook collapse behavior.

If and when we get to merging TOC (or another plugin) to core, and want to preserve git history (and make it easily accessible, unlike git subtree or subtree merges), here's one way to do it:

  1. Install git-filter-repo
  2. Clone the toc repo and use git filter-repo to move the commit history to the appropriate subdirectory:
    git clone git@github.com:jupyterlab/jupyterlab-toc.git
    cd jupyterlab-toc
    git filter-repo --to-subdirectory-filter packages/toc --tag-rename '':'@jupyterlab/toc@'
    cd ..
    Notice that the entire repo is now in the packages/toc directory, and all history happens there.
  3. Clone JupyterLab and merge the history
    git clone git@github.com:jupyterlab/jupyterlab.git
    cd jupyterlab
    git remote add toclocal ../jupyterlab-toc
    git fetch toclocal
    git checkout -b tocmerge
    git merge --allow-unrelated-histories toclocal/master

At this point, it probably makes sense to break up the package into the library and -extension package, integrate it with jlab core, etc.

I like git-filter-repo! It's much nicer and faster than the old git filter-branch!

Given the reservations expressed from those that have worked extensively with the code in question, I agree that we need to take a step back and reevaluate.

Thanks for the good feedback from the ones that have more historical info about the extension.

What I see today with this particular extension is that it is very popular on the user community and a lot of the cloud providers that have Jupyter based services deploy this extension by default. Having said that, and probably because it sort of works, it currently does not attract much interest from the dev side and somewhat important features and/or bugs are left unattended for a while (e.g. the collapse notebook feature).

My goal towards incorporating TOC into the core was to have more eyes on the dev side, get some prioritization and help from UX, etc, etc, and also make sure it's always in sync and available to all JLab releases.

From the list above, I feel some are nice to have while others might be blockers. Assuming that the most complained issue was related to collapsable cells and that is now optional and disabled by default, what would be the blocker issues that should be addressed before merging.

We had another discussion on TOC on today's JupyterLab dev meeting and the consensus seems to be that we should start working on the necessary steps to bring TOC to core.

One question that was brought up during the meeting is about collapsable cell feature and that is something we might need further discussion, where we need to have a final decision if we remove the functionality and leave an event to allow users to still have access to implement the capability or if we leave the current functionality disabled. We might opt for the first option as the "clean implementation" requires changes to core that might not come in the near future.

I will start working on moving the extension over the weekend.

This is now available in JupyterLab 3.0 master