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

Markdown integration #2444

Closed
motiz88 opened this issue Jul 10, 2017 · 15 comments
Closed

Markdown integration #2444

motiz88 opened this issue Jul 10, 2017 · 15 comments
Labels
lang:markdown Issues affecting Markdown locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:enhancement A potential new feature to be added, or an improvement to how we print something

Comments

@motiz88
Copy link
Contributor

motiz88 commented Jul 10, 2017

Hi everyone!

I've decided to dive into the Prettier source code with the intention of ultimately building full Markdown support into it. I can find my own way around the codebase OK for now, but I'd like to discuss here (given the truth of this tweet) some fundamental issues as I encounter them, to increase the chances of this actually landing in Prettier at some point.

Goal

A Markdown integration for Prettier, which, the way I envision it, involves formatting Markdown files and ideally also JS/CSS/etc code blocks within them.

Q: Is such a Markdown integration even desired?

Basic approach

Using remark as a parser and as a reference printer:

  1. Take a direct dependency on remark-parse to implement parser-remark.js (this part is straightforward).
  2. Fully port remark-stringify to Prettier's formatting primitives (and coding conventions) to implement printer-remark.js.
  3. Make stylistic tweaks to printer-remark.js once it has reached parity with remark-stringify.
  4. Multiparser??? - Haven't explored in any depth yet

Problems/questions

Passing user options to the parser/printer

remark supports several flavours of Markdown and takes various options; would we expose any of this to the user? (How?) The alternative is to essentially commit to a single flavour of Markdown, but that may be too opinionated to be viable.

A new document type for blockquotes

If we want to print Markdown blockquotes the "standard" pretty way - with hard wrapping and a > marker at the beginning of each line:

> This is a blockquote. Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aliquam hendrerit mi posuere lectus.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> This is a blockquote. Lorem ipsum dolor sit amet, consectetuer adipiscing
> elit. Aliquam hendrerit mi posuere lectus.

Then AFAICT we need a new document type to represent it and probably a different implementation of indent and align in doc-printer.js (supporting arbitrarily mixed indentation and non-space markers, including nested blockquotes etc). Can someone confirm this or point out something I've missed?

Multiparser architecture

I have not begun to tackle this at all, but would appreciate any thoughts about it nonetheless. My current thinking is that we can extract parts of the existing JS-centric "multiparser" for reuse in a Markdown multiparser, and potentially uncover interesting generalisations. Just realised the existing multiparser also does CSS in HTML; it's probably already general enough.


Thanks for a fantastic tool and thanks in advance for any help on this!

@vjeux
Copy link
Contributor

vjeux commented Jul 11, 2017

For the blockquote you can use the fill primitive built by @karl: https://medium.com/geckoboard-under-the-hood/adding-a-new-layout-strategy-to-prettier-8d33084c0f99

Instead of having hardline be the separator, you can have ifBreak(concat([hardline, "> "]), " ")

@karl
Copy link
Collaborator

karl commented Jul 11, 2017

@motiz88 If you need any help or pointers getting the fill primitive to work just give me a shout!

@motiz88
Copy link
Contributor Author

motiz88 commented Jul 11, 2017

@vjeux It will take me some digging to figure out what a solution using ifBreak would look like and whether it would cover all the use cases wrt arbitrary node nesting (e.g blockquote doesn't just contain a direct text child, can contain further indented content), but I'll try. Thanks!

In the mean time I've hacked in the new primitive I was after (tentatively markerBlock("> ", contents)), and I'll revisit it once I've covered a larger set of node types.

@karl
Copy link
Collaborator

karl commented Jul 11, 2017

@motiz88 I'd be really interested in seeing how the Markdown parsing works (specifically around the blockquotes). I think I can picture the kind of issues you might come across that would make using fill difficult/impossible but it would be great to see examples/code to anchor my thinking.

If you have some code already working please feel free to create a work in progress ([WIP]) PR. I'd love the ability to have a sneak peek at the code as you work on it 😀 I understand it may change a lot before it's final form.

@azz azz added the type:enhancement A potential new feature to be added, or an improvement to how we print something label Jul 11, 2017
@motiz88
Copy link
Contributor Author

motiz88 commented Jul 12, 2017

Not creating a PR just yet, as the code is nowhere near complete; But anyone wishing to have a super-early look is welcome to check out https://github.com/motiz88/prettier/tree/markdown or just the test snapshot file.

Again, just to clearly communicate where I am on this: Very few nodes are implemented right now, the tests don't all reflect the intended eventual behaviour, and overall correctness is - shall we say not guaranteed 😉

@karl It appears that fill is key to formatting Markdown, but I have struggled to get it to behave the way I want. For example, at the moment I selectively flatten certain subdocuments (e.g. paragraph contents) which feels like a hack.

@motiz88
Copy link
Contributor Author

motiz88 commented Jul 13, 2017

In fun news, curiosity got the better of me and I got the multiparser working, so my implementation now formats code within Markdown code blocks. I especially like this test of a prettified code block nested in a blockquote.

@motiz88
Copy link
Contributor Author

motiz88 commented Jul 13, 2017

At this point, I would love to get some sort of buy-in for this feature from Prettier's maintainers, as getting it into a PRable and mergeable state is going to take some time & effort - which I'm more than happy to put in, but not if, say, Markdown is simply out of scope for inclusion in Prettier and the PR would never be merged. Which is fair enough! Just tell me up front 😃

@jrwebdev
Copy link

I'd be very interested in this - we use Bitbucket Server at the company I work at and it doesn't wrap markdown when it's being viewed in PRs or have a rich diff mode like Github does. I'd be more than happy if the first release just solved the issue of wrapping the markdown - formatting the code within code blocks wouldn't be too important for us.

@leipert
Copy link

leipert commented Jul 14, 2017

Regarding a single flavor of markdown:

There is this initiative from several big shots in the markdown community: http://commonmark.org/

@azz
Copy link
Member

azz commented Jul 15, 2017

Formatting code inside Markdown would be amazing, I'm willing to help out with getting it merge-ready.

The alternative is to essentially commit to a single flavour of Markdown, but that may be too opinionated to be viable.

I think this is our best option. We haven't introduced any non-JS settings yet and I think it would be best to keep it that way.

@azz azz mentioned this issue Jul 15, 2017
@Andarist
Copy link

Aint sure what features do u plan as part of pretty formatting Markdown - but pretty printed tables would be incredibly cool!

Also great work on the multiparser 👏

@Graham42
Copy link

I'm not sure if prettier is should be a markdown formatter, feels like a stretch. However, I think any tool that is a markdown formatter should call prettier to format code code blocks.

As far as creating such a tool, my number 1 annoyance about peoples' READMEs is that they're poorly formatted for reading in a terminal. Markdown is first meant to be a plain text markup, it just conveniently is also used as the markup language for lots of stuff that outputs HTML (like GitHub).

I would love if a Markdown auto-formatter had output wrapped at 80 characters so that it could easily be viewed in a terminal.

@sorahn
Copy link

sorahn commented Aug 25, 2017

What about things like this?

https://github.com/thinkmill/react-markings

IMO it would be amazing if prettier formatted this like it formats styled components.

(I know that styled components are going to come up more than markdown, I'm just saying 😄 )

@azz
Copy link
Member

azz commented Aug 26, 2017

@sorahn If we land markdown support, it will be trivial to add support for react-markings!

@ikatyang ikatyang mentioned this issue Oct 1, 2017
8 tasks
@ikatyang
Copy link
Member

ikatyang commented Oct 6, 2017

Forgot to announce here, I've opened #2943 to address this issue, feedbacks welcome.

@azz azz closed this as completed in #2943 Oct 11, 2017
@ikatyang ikatyang added the lang:markdown Issues affecting Markdown label May 28, 2018
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Aug 26, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lang:markdown Issues affecting Markdown locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:enhancement A potential new feature to be added, or an improvement to how we print something
Projects
None yet
Development

No branches or pull requests

10 participants