zed
zed copied to clipboard
Markdown preview action
This PR implements a feature I was missing from VSCode: the ability to preview markdown files inside the editor. This allows users to preview README's and other markdown documents in a rich-text format instead of plain markdown.
Changes
- New
crates/markdown_previewcrate that registers themarkdown: open previewaction. crates/markdown_previewusescrates/rich_textto convert the active editor's text intoRichText. Note that markdown features not supported byRichTextwill also not be supported by this new action. See the below section "Missing features inrich_text".
Release Notes
- Added
Markdown: Open previewaction to partially close (#6789).
https://github.com/zed-industries/zed/assets/18583882/6c1a28fc-31f6-4294-a902-94cc7ec74448
Missing features in rich_text
Needed rich_text improvements:
- [ ] Support inline code blocks (
my_code) - [ ] Make code blocks ~display-block instead of ~inline
- [ ] Different heading levels should have different font sizes
- [ ] Support task lists (
- [ ]and- [x])
Looks great🥳
This is fantastic! We've been needing Markdown preview for a long time.
Did a quick test - exciting! One thing to note, checkmarks don't seem to be rendering in tasks:
@JosephTLyons that's correct; I'll update the PR description to reflect that. Anything that isn't implemented in crates/rich_text won't be visible in the markdown preview. What's Zed's preferred approach to shipping features like this? Land and expand? Or is this something you'd want taken care of before considering merging?
Ok, it was a pre-existing issue here - gotcha. And I'm not sure, we've taken both approaches in the past:
- Polishing heavily before release
- Releasing with the expectation of following with some polish
Not sure what the right answer is here. I'll let @maxbrunsfeld or @mikayla-maki weigh in...
We’re still collectively working out what it means to open source and how to interface with the community, so consider everything I say to be v0.1 and subject to change 😄
Generally, our policy internally at Zed has been to merge things once they’ve passed our collective quality bar and to prefer scrapping or postponing a feature if we can’t do it right. Right meaning fast, complete, and with a buttery smooth UI/UX experience. We don’t want to leave a lot of half baked features lying around in the source waiting for someone to remember that they exist. Obviously, we’re not always 100% on this value and some features are too large to ship in a single PR. But what we merge should never break main for our stable users and we strive to uphold that to the best of our ability at the time.
Under this policy, I’d say that the scrolling and live updating features are essential to shipping this. And further I’d say we need to figure out the UX implications of how we actually show the preview. Right now it’s implemented as its own item (tab), but I think it might be more useful as a split view? But then how does that feel with an already split view? I feel like we’d want to create a ‘SplitItem’ struct that can contain two ‘Item’s under it, so that the preview and the editor view are treated as the single document they actually are. Getting this right would also block shipping this PR.
Checkboxes are a bit of an edge case here. IMO, we could ship without them because the back-and-forth editing implied by checking them sounds like a lot more complexity and it doesn’t significantly add to the value of having markdown previews.
I hope that clarifies where my head is at, though if Max has different opinions I’d defer to his.
Thanks for clarifying @mikayla-maki!
I took care of my final TODOs: scrolling and live updating. I'm at a stopping point with what I originally needed out of a markdown preview.
Since you mentioned the UX around opening the preview is unclear, I'll leave this PR in draft for now. Feel free to close it if your team decides to move in a different direction with this feature.
I appreciate the team's quick feedback and look forward to having this feature (whether or not it comes in from this PR!)
Thank you for this PR @kierangilliam, if you or someone else wants to continue to adding the SplitItem struct I mentioned (or some other approach) we'd be happy to chat about it at one of our fireside hacks, or on a channel, or on discord or some such. For right now though, we'll check in about what to do here during our work week. Maybe I'll just implement that onto main and then you can use it for this PR 🤔. Regardless, thank you for adding this feature!
@kierangilliam : The images and tables are displayed correctly?
@Angelk90
I carried out some tests with tables and images. In the tests carried out, it did not work.
I believe that for a V1 it is ok.
What do you think about this improvement coming after his PR?
Test with image:
Test with table:
@juninhopo @Angelk90
I'm working through an alternate implementation here (6958) which has basic support for tables. This markdown renderer will also make it easier to support images in the future.
I attempted to add the SplitItem struct I described, and it turned out to be a bit more complicated than I expected. I think this should be fine to ship without it :)
This PR seems to have been superseded by https://github.com/zed-industries/zed/pull/6958