Markdown preview action #6958
Markdown preview action #6958
Conversation
cla-bot
bot
added
the
cla-signed
There was a problem hiding this comment.
Coming back from #6886 (reply in thread) with all the answers, but did not browse through the entire PR yet (anybody else is welcome to do so, I plan to do it tomorrow only).
Quick question: this PR looks better than #6883 so I wonder if we could close the old one? Or is there anything of value there still?
kierangilliam
changed the title
|
@kierangilliam This is looking good! I left a few comments, but I think we're getting close to being able to merge this. |
|
This is looking really good! I was poking around to see if we could cache the markdown parsing or keep from serializing the buffer on every keystroke but it seems like both would be a whole project unto themselves. I think this can ship without either getting in the way too much :) |
There was a problem hiding this comment.
I see a few things that bother me still:
-
minor: regular text font size seems to be even bigger in the render, bloating it a bit
-
minor: markdown headers in the code have the color, but the rendered version does not
-
major: single line code blocks do not seem to rendered at all (but multiline are fine)
- major: inner markdown links do not work, erroring (HTTP URLs work just fine)
- major: there's no scrolling happening (presumably, also related to the previous bullet).
If you open the syntax tree view, you can notice both the editor and the view are scrolled when you click in either of the view/editor.
I think we need to have something even more advanced, presumably scrolling always, so both the editor and the render never show different areas.
On the one hand, it's been quite a while we kept this PR unmerged and it got a good progress (for the first try, especially).
On the other hand, it's clearly not finished and needs more work and if we just merge the PR without fixing those, we'll get more tech debt right away.
So, I would try and fix the code blocks highlighting and disable the inner link clicking for now as a minimum before merging this — all other things seem good to have, but maybe later.
|
Did a bit of following up on the inline code highlighting issue, as I'd love to have this feature, and it seems there's a completely separate kind of markdown parsing that Zed also uses, in Thanks for biting off such a chunk of complexity @kierangilliam, if you ever want to talk about this PR or pair on it feel free to drop by a fireside hack (and we can set one up) or jump into our discord :) |
|
I have more work to do so I'm moving this back to draft. The feedback is very appreciated <3 To address @SomeoneToIgnore comments:
Inline code blocks are not supported by The more we talk about this feature, the more I'm convinced that I should break away from using
Yikes. I should've caught that. I'll probably have to move off of using
I don't have an opinion on what the color should be so I'm open to making this change! Is there a public style guide that I can refer to in the future?
I don't think I'll have time to address this one in this PR but I'll keep it in mind for a future contribution! |
@mikayla-maki thanks for pointing this out. This seems to be roughly equivalent to |
|
@kierangilliam roger about the highlights, no worries about them now then. |
|
Why dont we support features like this in some kind of plugin system? Or can we do all things user need in a single app build? Only curious. |
|
Because there's no plugin system to start with, and people seem to want features nonetheless. Hence, for now we try to keep the balance between rejecting everything and keeping the good bits in. |
Done. I'll follow up in another PR with more markdown support while trying to hit some of the other issues you all raised @SomeoneToIgnore. |
Just a small improvement as a follow up to @kierangilliam great work on #6958 Rendering a table specified like this: ```markdown | Left columns | Center columns | Right columns | | ------------- |:--------------:| -------------:| | left foo | center foo | right foo | | left bar | center bar | right bar | | left baz | center baz | right baz | ``` Does now look like this (notice the cell alignments):  Release Notes: - N/A
This PR improves support for rendering markdown documents. ## After the updates https://github.com/zed-industries/zed/assets/18583882/48315901-563d-44c6-8265-8390e8eed942 ## Before the updates https://github.com/zed-industries/zed/assets/18583882/6d7ddb55-41f7-492e-af12-6ab54559f612 ## New features - @SomeoneToIgnore's [scrolling feature request](#6958 (review)). - Checkboxes (`- [ ]` and `- [x]`) - Inline code blocks. - Ordered and unordered lists at an arbitrary depth. - Block quotes that render nested content, like code blocks. - Lists that render nested content, like code blocks. - Block quotes that support variable heading sizes and the other markdown features added [here](#6958). - Users can see and click internal links (`[See the docs](./docs.md)`). ## Notable changes - Removed dependency on `rich_text`. - Added a new method for parsing markdown into renderable structs. This method uses recursive descent so it can easily support more complex markdown documents. - Parsing does not happen for every call to `MarkdownPreviewView::render` anymore. ## TODO - [ ] Typing should move the markdown preview cursor. ## Future work under consideration - If a title exists for a link, show it on hover. - Images. - Since this PR brings the most support for markdown, we can consolidate `languages/markdown` and `rich_text` to use this new renderer. Note that the updated inline text rendering method in this PR originated from `langauges/markdown`. - Syntax highlighting in code blocks. - Footnote references. - Inline HTML. - Strikethrough support. - Scrolling improvements: - Handle automatic preview scrolling when multiple cursors are used in the editor. - > great to see that the render now respects editor's scrolls, but can we also support the vice-versa (as syntax tree does it in Zed) — when scrolling the render, it would be good to scroll the editor too - > sometimes it's hard to understand where the "caret" on the render is, so I wonder if we could go even further with its placement and place it inside the text, as a regular caret? Maybe even support the selections? - > switching to another markdown tab does not change the rendered contents and when I call the render command again, the screen gets another split — I would rather prefer to have Zed's syntax tree behavior: there's always a single panel that renders things for whatever tab is active now. At least we should not split if there's already a split, rather adding the new rendered tab there. - > plaintext URLs could get a highlight and the click action ## Release Notes - Improved support for markdown rendering.
…#7345) This PR improves support for rendering markdown documents. ## After the updates https://github.com/zed-industries/zed/assets/18583882/48315901-563d-44c6-8265-8390e8eed942 ## Before the updates https://github.com/zed-industries/zed/assets/18583882/6d7ddb55-41f7-492e-af12-6ab54559f612 ## New features - @SomeoneToIgnore's [scrolling feature request](zed-industries#6958 (review)). - Checkboxes (`- [ ]` and `- [x]`) - Inline code blocks. - Ordered and unordered lists at an arbitrary depth. - Block quotes that render nested content, like code blocks. - Lists that render nested content, like code blocks. - Block quotes that support variable heading sizes and the other markdown features added [here](zed-industries#6958). - Users can see and click internal links (`[See the docs](./docs.md)`). ## Notable changes - Removed dependency on `rich_text`. - Added a new method for parsing markdown into renderable structs. This method uses recursive descent so it can easily support more complex markdown documents. - Parsing does not happen for every call to `MarkdownPreviewView::render` anymore. ## TODO - [ ] Typing should move the markdown preview cursor. ## Future work under consideration - If a title exists for a link, show it on hover. - Images. - Since this PR brings the most support for markdown, we can consolidate `languages/markdown` and `rich_text` to use this new renderer. Note that the updated inline text rendering method in this PR originated from `langauges/markdown`. - Syntax highlighting in code blocks. - Footnote references. - Inline HTML. - Strikethrough support. - Scrolling improvements: - Handle automatic preview scrolling when multiple cursors are used in the editor. - > great to see that the render now respects editor's scrolls, but can we also support the vice-versa (as syntax tree does it in Zed) — when scrolling the render, it would be good to scroll the editor too - > sometimes it's hard to understand where the "caret" on the render is, so I wonder if we could go even further with its placement and place it inside the text, as a regular caret? Maybe even support the selections? - > switching to another markdown tab does not change the rendered contents and when I call the render command again, the screen gets another split — I would rather prefer to have Zed's syntax tree behavior: there's always a single panel that renders things for whatever tab is active now. At least we should not split if there's already a split, rather adding the new rendered tab there. - > plaintext URLs could get a highlight and the click action ## Release Notes - Improved support for markdown rendering.
|
I am a recent user of zed and I have encountered a problem with markdown preview feature. It basically doesn't render the images. There is only a blank space. 
|
This PR adds markdown rendering to Zed.
crates/markdown_previewexposes a new "markdown: open preview" action. This allows users to preview a rendered version of whichever markdown file they are currently viewing.zed.-.md.preview.3.mov
This PR extends the work done in
crates/rich_textto render markdown to also support:Release Notes
Markdown: Open previewaction to partially close (#6789).Known issues that will not be included in this PR
rich_text)- [ ]and- [x])Original PR: #6883