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

More provider options: lifecycle hooks, math typesetting, link replacing #14

Open
bollwyvl opened this issue Nov 12, 2020 · 15 comments
Open

Comments

@bollwyvl
Copy link
Contributor

From #10 (comment):

  • the core extension can
  • allow disabling built-in math typesetting (and or other features, like link replacement)
  • provide access to lifecycle hooks to plugins for when those features would have happened

If a plugin wants to "step up" and overload an existing feature, it should be possible to do so. One certainly wouldn't want mulitple link replacers, math typesetters, or whatever.

Semi-concretely:

  • expand options to be something like...
options: {
  MarkdownIt: ...
  renderMarkdown: (something) => Promise<Partial<Omit<renderMarkdown.IRenderOptions, 'host', 'source'>>>
}
  • if multiple things try to register options, it's probably broken

  • take over more of renderMarkdown

    • at each of the stages, allow each plugin to register things... so maybe like...
      • beforeMarkdownIt
      • afterMarkdownIt
      • beforeLatex
      • afterLatex
      • beforeHTML
      • afterHTML
  • Each of these events would probably need a rank or weight for different plugins.

Some things that could be built with this, depending on how much context was given to the plugins:

  • given just above, a server-based latex renderer
    • put a placeholder in (or the previous/cached math), wait for it to render
  • given the current widget, a jupyter widget replacer
    • find specially-flavored links/images, e.g. ![](widget://foo) replace them with a widget
    • some lovely ones like tanglejs would make my day
@rowanc1
Copy link

rowanc1 commented Nov 26, 2020

Is this repo using a package to figure out the markdown math parsing yet? I couldn't find any in your dependencies. I am working on https://github.com/executablebooks/markdown-it-myst at the moment and am starting to run into problems with all of the current -textmath -math -mathjax markdown-it extensions. Wondering if you had done that here yet? If not, maybe a collaboration target that is relevant to the jupyter ecosystem? I am happy to push on it ...!

Also just a note on the:

some lovely ones like tanglejs would make my day

I am working on a few of these type of components over at https://iooxa.dev and will eventually work on integrating with Jupyter. :)

@bollwyvl
Copy link
Contributor Author

using a package to figure out the markdown math parsing

No, it presently delegates to the in-built MathJax2 renderer. By adding the flags suggested here, it would be possible to override this behavior... but unless you're providing something really fancy, I'd make it possible to opt out of that opinion, if it's getting embedded inside a larger system (e.g. JupyterLab).

lovely ones like tanglejs
working on a few of these type of components

Best of luck!

@agoose77
Copy link
Owner

Thinking about this more, I'd probably suggest implementing a custom LaTeX typesetter extension rather than using lifecycle hooks to implement a new math renderer, unless it needed access to some of the other stages too. @rowanc1 what issues are you currently experiencing with the default / KaTeX math renderers?

@rowanc1
Copy link

rowanc1 commented Nov 30, 2020

Do you have any problems with the typesetter in places like $x *y* z$? Or using underscores etc.?

The extensions that I am looking to use/make disable the markdown parsing inside of known math strings, and then also on the MyST side we are interested in the referencing and numbering responsibility (which can sometimes be delegated to mathjax). The problem that I have with the current extensions out there is that they do the rendering, or pre-rendering as well as the parsing.

So not looking to create a new renderer, just to recognize the math when parsing the markdown and create a standard output that mathjax/katex can then render. With cross references and labelling, we need to know at parse time about the equations/inline math.

Maybe that clarifies things?! :)

@agoose77
Copy link
Owner

The math rendering is currently taken care of by removing the math entirely from the markup, and then restoring it later once Markdown rendering has completed:
https://github.com/jupyterlab/jupyterlab/blob/master/packages/rendermime/src/latex.ts

If you need more, then we would need to implement the lifecycle hooks as described in this PR :)

@rowanc1
Copy link

rowanc1 commented Nov 30, 2020

So interesting, I wasn't aware of how Jupyter was doing this. This makes a lot more sense now. Thanks for the pointer!

@agoose77
Copy link
Owner

agoose77 commented Jun 21, 2022

@bollwyvl I'm thinking this stuff through at the moment, in order to support MathJax3 which really wants an HTMLElement to work with, and I'd appreciate your input if you've the cycles free.

I'm working on this as I need it for writing my thesis, so my priorities are:

  • do as little as possible to get something deliverable
  • do it not-wrong first time ;)

Of the features you list above, I really think we want as first-deliverables:

  • Ignore the ILatexTypeSetter interface entirely (it doesn't give us enough control over where the math comes from, and I'm not comfortable with wrapping math in $$ so that it gets picked up by the typesetter. Maybe I'm too purist.)
  • Add a postRender hook.
    • Add new postRenderHook: Promise<IPostRenderHook> member to IPluginProvider interface. IRenderHook interface has two members:
      • postRender(node: HTMLElement): Promise<void>
      • rank: number
    • Change getMarkdownIt for getMarkdownRenderer that has
      • render(content: string): string
      • postRender(node: HTMLElement): Promise<void>
      • markdownIt: MarkdownIt (getter)
  • Manager now just builds the IRenderer object that gets passed to RenderedMarkdown etc.

I've implemented this in #60

You can see an example plugin in https://github.com/agoose77/jupyterlab-markup/blob/7b4bee5be777cb4b05a19ad65483b40cb3dad015/src/builtins/post-render.ts

I really want to keep the API small and opinionated, so I opted to remove the getMarkdownIt method in favour of getRenderer. If we want to be "good", I'll restore getMarkdownIt and just deprecate it, as the new postRenderHook implementation is backwards compatible.

I haven't added any priority to the main plugin.use mechanism. I suspect we might want that also, any thoughts?

@bollwyvl
Copy link
Contributor Author

support MathJax3

someone with time should really try to get mathjax 3 into lab 4, as it's one place where it doesn't benefit much having lots in implementations. i don't have a lot of feedback, as "don't do much" and "do it right" are usually at odds: at this point, as long as its versioned such that changing the API doesn't break the expectations of jupyterlab-myst for existing users, you're probably fine.

getMarkdownIt

Sure, whatever: just can't get too fancy, as for this to be useful, extenders need a real handle to the actual shared library singleton because dirty internal state.

any priority to the main plugin.use mechanism.

I don't really follow...

@agoose77
Copy link
Owner

In #60 I've gone a slightly different route to #14 (comment) and just disabled the math rendering from RenderedMarkdown. Now, math blocks need to satisfy the .math selector, which is used by the postRender hook of an "adaptor" plugin that invokes ILatexTypesetter.typeset. This means we can use https://github.com/jupyterlab/jupyter-renderers/blob/master/packages/mathjax3-extension, which suits me fine!

Yeah, to be honest I've had so little time to plan this out, I just wanted a separate pair of eyes, so thanks for taking a look.

Sure, whatever: just can't get too fancy,

For sure.

I don't really follow...

I have only added a priority system to enforce ordering on the postRender hook. I haven't done the same to influence the order in which MarkdownIt.use is invoked. There is already an ordering mechanism in the rules engine, but if two plugins use the same rules, I imagine it's "first-wins".

@bollwyvl
Copy link
Contributor Author

use mathjax3-extension

Great, this is an ideal use of the system.

influence the order in which MarkdownIt.use is invoked

Ah. seems like if you're adding priority, it would make sense to add in both places... conflicts seem like they could be a very real problem.

@agoose77
Copy link
Owner

agoose77 commented Jun 22, 2022

I think you're right. It's an easy thing to add, and there are no real downsides of doing it.

I've added rank to the pluginProvider (which is the plugin rank), and the other two lifecycle hooks:

  • postRenderHook
  • preParseHook

I think these two hooks should cover most use cases. The preParseHook allows the user to modify the source before parsing. I really don't know why we'd want to do this, but it might be useful.

I haven't figured out how to do configuration for the lifecycle hooks yet. Part of me things we just make the extensions handle this themselves with a schema as it's a fairly "advanced" feature.

@agoose77
Copy link
Owner

@bollwyvl one area I'm unsure on is whether to make the lifecycle hooks stateful. I have two API designs at present:

  • add new factory async function to IPluginProvider, which returns
export interface IComponent extends IRanked {
    plugin?: IPlugin;
    pluginOptions?: any[];
    markdownItOptions?: any[];
    preParseHook?: IPreParseHook;
    postRenderHook?: IPostRenderHook;
  }
  • extend existing IPluginProvider with new hooks (see main branch)

I feel like having state here might be useful, but at the same time I like the fact that it's not stateful: hooks can't communicate with each other or the main parser. Do you have any feelings here?

@agoose77
Copy link
Owner

agoose77 commented Jun 23, 2022

I think I'm letting perfect become the enemy of good enough.

There are a lot of unknowns about how people use jupyterlab-markup, and what extension authors want. I'm opting to just extend the current API for backwards compatibility, because I anticipate a bigger change being required later down the road for e.g. supporting IMarkdownParser (which would be a weak version of MarkdownIt) and whether users should be able to configure the plugin ranks.

Hence:

export interface IPluginProvider extends IRanked {
  /**
   * A unique identifier for the plugin, usually the name of the upstream package
   */
  id: string;
  /**
   * A human-readable name for the plugin
   */
  title: string;
  /**
   * A short description for the plugin
   */
  description: string;
  /**
   * URLs for learning more about the plugin with human-readable keys
   */
  documentationUrls: { [key: string]: string };
  /**
   * Short usage examples of any new syntax with human-readable keys
   */
  examples?: { [key: string]: string };
  /**
   * A lazy provider of the plugin function and plugin options
   */
  plugin?(): Promise<[IPlugin, ...any]>;
  /**
   * Additional options to pass to the MarkdownIt constructor
   */
  options?(widget: RenderedMarkdown): Promise<{ [key: string]: any }>;
  /**
   * A lazy provider of a post-render hook
   */
  preParseHook?: IPreParseHook;
  /**
   * A lazy provider of a post-render hook
   */
  postRenderHook?: IPostRenderHook;
}
export interface IPreParseHook extends IRanked {
  /**
   * Pre-parsing callback
   */
  preParse(content: string): Promise<string>;
}
export interface IPostRenderHook extends IRanked {
  /**
   * Post-rendering callback
   */
  postRender(node: HTMLElement): Promise<void>;
}

@bollwyvl
Copy link
Contributor Author

Some thoughts:

  • when talking about "plugins", maybe specify MarkdownIt plugin (vs JupyterLab plugin or jupyterlab-markup plugin)
  • the descriptions of the hooks could be clearer regarding their signatures: e.g. preParse transforms the content, postRender modifies in-place.
  • maybe break the hooks into a sub-interface, e.g. you could even get a little fancy with a generic:
markup.register({
  id: 'noop',
  // ...
  hooks: {
    preParse: { rank: 1, execute: async (text) => text }
  }
})
export interface IHook<T, U> extends IRanked {
  execute(input: T): Promise<U>;
}

export interface IPluginHooks {
  /** transform the markdown content before any parsing occurs */
  preParse?: IHook<string, string>
  /** modify the container DOM element in-place */
  postRender?: IHook<HTMLElement, void>
}

export interface IPluginProvider extends IRanked {
  // ...
  hooks?: IPluginHooks;
}

@agoose77
Copy link
Owner

Hmm, I like the idea of putting these into an interface. I'd been toying with it on and off, so thanks for the nudge. I'm aware that I don't have as much time to get this right as I normally would, so the extra input's been useful.

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

No branches or pull requests

3 participants