-
Notifications
You must be signed in to change notification settings - Fork 13
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
Comments
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:
I am working on a few of these type of components over at https://iooxa.dev and will eventually work on integrating with Jupyter. :) |
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).
Best of luck! |
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? |
Do you have any problems with the typesetter in places like 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?! :) |
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: If you need more, then we would need to implement the lifecycle hooks as described in this PR :) |
So interesting, I wasn't aware of how Jupyter was doing this. This makes a lot more sense now. Thanks for the pointer! |
@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:
Of the features you list above, I really think we want as first-deliverables:
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 I haven't added any priority to the main |
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
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.
I don't really follow... |
In #60 I've gone a slightly different route to #14 (comment) and just disabled the math rendering from 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.
For sure.
I have only added a priority system to enforce ordering on the |
Great, this is an ideal use of the system.
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. |
I think you're right. It's an easy thing to add, and there are no real downsides of doing it. I've added
I think these two hooks should cover most use cases. The 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. |
@bollwyvl one area I'm unsure on is whether to make the lifecycle hooks stateful. I have two API designs at present:
export interface IComponent extends IRanked {
plugin?: IPlugin;
pluginOptions?: any[];
markdownItOptions?: any[];
preParseHook?: IPreParseHook;
postRenderHook?: IPostRenderHook;
}
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? |
I think I'm letting perfect become the enemy of good enough. There are a lot of unknowns about how people use 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>;
} |
Some thoughts:
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;
} |
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. |
From #10 (comment):
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:
if multiple things try to register options, it's probably broken
take over more of renderMarkdown
beforeMarkdownIt
afterMarkdownIt
beforeLatex
afterLatex
beforeHTML
afterHTML
Each of these events would probably need a
rank
orweight
for different plugins.Some things that could be built with this, depending on how much context was given to the plugins:
![](widget://foo)
replace them with a widgetThe text was updated successfully, but these errors were encountered: