[WIP] beautify code blocks in markdown

[kachkaev]

What does this implement/fix? Explain your changes.

I needed to format code blocks inside markdown documents, but could not find any solutions for doing this. After some experimentation with atom-beautify I ended up with a sketchy prototype that roughly did what I was looking for.

The idea is simple: after a markdown file has been beautified as usual, fenced code blocks such as starting with ```js are extracted from the cleaned text and then each of them is passed through another round of beautify. Once all the blocks are ready, the parent markdown document is patched and the final result is returned.

To avoid breaking changes and unwanted side-effects, code block formatting is hidden behind a new option, which is off by default.

I'm submitting this PR mainly to start a discussion of this new potential functionality, so the original implementation does not pretend to meet the community standard. There are still a few issues, even if we put the ‘beauty’ of my amateur coffeescript aside:

• There is no way to only format code blocks and not the markdown text itself
• The language specified after ``` is internally mapped into file extension, which then determines the grammar to use. This means that ```js and ```py work, while ```javascript and ```python do not. UPD: This should be solved by the introduction of src/beautifiers/markdown-blocks/languages.coffee
• Tidy Markdown beautifier creates problems by converting ```js to ```javascript and not working well with block options such as ```js {style=special}. Remark seems to be OK with all this.
• When beautification implies missing statements such as auto-appending of module Main exposing (..) in Elm, the beautified code contains more changes than it needs to. This might require some flag to be passed down to all code beautifiers to tell them that the text is a fragment, not the entire file. They'll decide what to do.
• The option to beautify code blocks is currently all or nothing. Perhaps it'll be useful to say something like only beautify js and python blocks, keep others untouched.
• Not all types of fenced blocks are currently detected because of how gfm-code-blocks works (I rely on this dependency to find code inside markdown).

What do you guys think about this idea in general? What would you recommend to re-implement and how? I'm curious to hear your feedback and will be happy to work on a proper implementation if the proposed idea has a chance to land into your awesome Atom package!

Checklist

Check all those that are applicable and complete.

• Merged with latest master branch
• Regenerate documentation with npm run docs
• Add change details to CHANGELOG.md under "Next" section
• Added examples for testing to examples/ directory
• Travis CI passes (Mac support)
• AppVeyor passes (Windows support)

[kachkaev]

@szeck87 @Glavin001 what do you think of this idea?

[kachkaev]

@szeck87 @Glavin001 what do you think of this idea?

[Glavin001]

This is an awesome idea! Thank you for contributing! I made a few comments on the code itself. Please let me know if you have any questions.

@szeck87: would be great to get your input as well. Thanks!

[Glavin001]

.then((t) -> resolve(t))
.catch((e) -> reject(e))

This can be:

.then(resolve)
.catch(reject)

[Glavin001]

.then(resolve)
.catch(reject)

[Glavin001]

It appears all of the branches lead to a beautifyBlockPromises.push.

I think using Array.map would be better for this. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map

[kachkaev]

Good point! I was not pushing nulls in the very first implementation, so did not consider Array.map as an option here.

[Glavin001]

I do not like having side effects within Promises (i.e. cleanMarkdown is global variable and changed, instead of resolving the new value in the promise to be used later with .then(newValue)). Instead, please consider returning within the Promise itself. Array.reduce would be good here ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce ). For example:

# ...
Promise.all(beautifyBlockPromises).then((cleanCodeBlocks) ->
  cleanCodeBlocks.reverse((b) -> !!b).reduce((cleanMarkdown, codeBlock) -> (
         if (codeBlock)
           return cleanMarkdown.substring(0, codeBlock.originalStart) + codeBlock.block  cleanMarkdown.substring(codeBlock.originalEnd)
        else
           return cleanMarkdown
       ), cleanMarkdown)
).then(resolve)

Where ).then(resolve) is shorthand for ).then((cleanMarkdown) -> resolve(cleanMarkdown)).

[kachkaev]

I addressed this in a new commit – thanks for the hint. The code looks cleaner indeed!

[Glavin001]

This is a fairly large code block. Please break it down into logically grouped functions.

For example:

function languageForCodeBlock(codeBlock) {
  return codeBlock.lang.match(/^[\w\d]+/)?[0];
}

And

function allOptionsForLanguage(codeLanguage) {
  filePath = 'fakepath.' + codeLanguage
  grammar = atom.grammars.selectGrammar(filePath, code)
  grammarName = grammar.name
  return beautifier.getOptionsForPath(filePath)
}

etc.

Anything you can think of to make the code more readable and maintainable 😄 .

[kachkaev]

I addressed the first example by creating a new file that only focuses on language detection (file extension detection, to be precise). Extracting allOptionsForLanguage ended up being more challenging so I cancelled it. The problem is that the beautifier requires not only allOptions, but also the grammar and the path (beautifier.beautify(code, allOptions, grammarName, filePath)). Besides, the instance of beautifier is being used to extract options, meaning that this object would have to become a function argument. Too much wires to justify code splitting IMO.

[szeck87]

What's the reasoning behind creating this file to use as beautifier versus just Beautifier = require('../beautifier')?

[kachkaev]

An instance of beautifier used to be created inside src/beautify.coffee, which is the main file in the module. We now require this object to beautify code blocks inside markdown, but we don't want to create circular file references. I moved the creation of the beautifier into this new file and required it inside src/beautify.coffee as well as src/beautifiers/markdown-blocks/index.coffee.

The existing src/beautifiers/beautifier.md exports a class, not an acting object instance.

[kachkaev]

Awesome, thanks for the feedback guys! Will address your comments now 👍

[kachkaev]

Awesome, thanks for the feedback guys! Will address your comments now 👍

[kachkaev]

Thanks again for looking at this PR @Glavin001 and @szeck87! Happy to hear more feedback from you!

[kachkaev]

Thanks again for looking at this PR @Glavin001 and @szeck87! Happy to hear more feedback from you!

[Glavin001]

Overall this is looking good! I think all this needs are some test samples add to examples/ and add a .jsbeautifyrc file to configure beautifyCodeBlocks option to true.

[Glavin001]

I don't think you meant to pass the logger to tidyMarkdown: https://github.com/slang800/tidy-markdown/blob/master/src/index.coffee#L234

Maybe to beautifyCodeBlocks instead?

[kachkaev]

@Glavin001 will you have time to finish these changes by a chance? I'm afraid I'll be quite busy for the next several weeks. Feel free to take over this PR and merge it!

[kachkaev]

@Glavin001 will you have time to finish these changes by a chance? I'm afraid I'll be quite busy for the next several weeks. Feel free to take over this PR and merge it!

[kachkaev]

Given that Prettier is also a part of atom-beautify now, that beautifier will also need to be patched to support code block beautification. If anyone can help me with this by patching this PR, that'd be awesome! If you can't edit the PR directly, feel free to commit elsewhere and I'll add that commit to branch kachkaev:beautify-code-blocks-in-markdown.

[kachkaev]

Given that Prettier is also a part of atom-beautify now, that beautifier will also need to be patched to support code block beautification. If anyone can help me with this by patching this PR, that'd be awesome! If you can't edit the PR directly, feel free to commit elsewhere and I'll add that commit to branch kachkaev:beautify-code-blocks-in-markdown.

[szeck87]

@kachkaev what about this one? Do you anticipate a plugin being created for Prettier?

[szeck87]

@kachkaev what about this one? Do you anticipate a plugin being created for Prettier?

[kachkaev]

Prettier supports formatting of fenced code blocks in markdown files already. Some bits a still flux, I’ve already managed to use their plugin API to format Elm code in markdown (this was my intention when submitting this PR).

All works pretty well!

[kachkaev]

Prettier supports formatting of fenced code blocks in markdown files already. Some bits a still flux, I’ve already managed to use their plugin API to format Elm code in markdown (this was my intention when submitting this PR).

All works pretty well!