improvement: Realtime Markdown rendering #2138
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
wasimsandhu
changed the title
wasimsandhu
changed the title
frontend/src/core/codemirror/cm.ts
Outdated
| @@ -87,6 +88,8 @@ export const setupCodeMirror = (opts: CodeMirrorSetupOpts): Extension[] => { | |||
| // Cell editing | |||
| cellMovementBundle(cellId, cellMovementCallbacks, hotkeys), | |||
| cellCodeEditingBundle(cellId, cellCodeCallbacks, hotkeys), | |||
| // Markdown autorun | |||
| markdownAutoRunExtension(cellMovementCallbacks), | |||
There was a problem hiding this comment.
Would this be better in the MarkdownLanguageAdapter class? Better separation of concerns and no need to do the if check
|
Nice! Might be worth seeing how this interacts with the VSCode plugin which already does this |
|
@dmadisetti - its essentially the exact same logic, but they won't overlap or effect each other. |
|
@wasimsandhu , this is awesome!! |
There was a problem hiding this comment.
I don't remember if we "debounce"/deduplicate run requests in the backend. I think we might not, in which case if the kernel were busy while you were typing in a markdown cell, it's possible that hundreds of run requests would get queued and then run serially.
Might be something to test -- do time.sleep(10) in one cell, then start typing a bunch of text in a markdown cell and see what happens. I can help improve the BE if this is indeed an issue.
|
Ah that's a good point. Let me check it out and report back. @akshayka update: 
|
|
@wasimsandhu - its ok if the markdown is queued and doesn't update. but the worry is if after the sleep is done, will it run 30 requests (one for each char) through the kernel or 1. ideally its 1, if that is not the case, I will dedupe RunRequests in another PR. |
|
@mscolnick Ahhh gotcha. If it's alright, I'd love to take a stab at it |
|
Totally - go for it! Thanks |
|
Just finished implementing @akshayka's de-duplication logic only to realize run requests were not getting duplicated to begin with... the markdown cell is stale while other cells are running, and it does not autorun after the I guess I must have hallucinated it the first time I observed it? I was positive I saw tons of duplicated requests, but now I'm unable to reproduce it lol. Can someone test on their machine and confirm that requests are not being duplicated as the branch currently stands? I have my deduplication logic stashed and ready to push in case I'm wrong. |
|
Looks like I misled myself. After trying some different things, I realized that:
This is the reason I "hallucinated" duplicated requests when I was testing... just clicking a few times after the blocking cell run sent 10-15 requests all at once. Updated extensions.ts so that a run only occurs if the cell's code was changed 😅 |
|
Sounds good. I will review tomorrow |
|
/marimo create-test-release |
|
🚀 Starting test release process... |
|
@wasimsandhu i just tested it. you are correct, it works as expected and we aren't sending additional requests |
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.8.8-dev19 |
📝 Summary
Completes #2137.
🔍 Description of Changes
📋 Checklist
📜 Reviewers
@akshayka OR @mscolnick