-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
Set up MarkdownCodeBlockFormatter #1419
Conversation
Thank you for contributing to Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:
Automated comment created by PR Commenter 🤖. |
@@ -173,7 +176,7 @@ and Erlang types. To create a plt with sensible defaults run: | |||
$ mix dialyzer --plt | |||
``` | |||
|
|||
Finally it can be run with: | |||
Finally, it can be run with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a formatter feature 😛
@@ -59,6 +61,8 @@ if age >= 16, do: "beer", else: "no beer" | |||
|
|||
This may look like `if` accepts two arguments, but the `do:` and `else:` pair is actually a single argument - a keyword list. The same code could be written as: | |||
|
|||
[]: # (elixir-formatter-disable-next-block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where desired, formatting can be turned off. For example here we want to show off a non-standard way to format this code to prove a point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this looks pretty good!
My only thought is I don't know what is standard in the markdown standard for those formatter directives to ignore some blocks. But it works for me!
Part of the problem is that there is no one Markdown standard 😞 This syntax:
It's an abused reference-style link. More generically, it's:
Now, I don't know if all Markdown implementations support reference-style links, but Exercism definitely depends on them. I'm pretty sure that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outstanding! I love formatters :)
I left a couple of questions.
@@ -46,7 +49,8 @@ We can achieve the same result by prepending an element to the reversed list, an | |||
[1, 2, 3] ++ [4] ++ [5] ++ [6] | |||
|
|||
# Prepend to the start of a list (faster, due to the nature of linked lists) | |||
[6 | [5 | [4 | [3, 2, 1]]]] # then reverse! | |||
[6 | [5 | [4 | [3, 2, 1]]]] | |||
# then reverse! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this comment go at the bottom?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it manually because the "reverse" part is not part of this code. The code only shows list creation, and then it's implied that you need to reverse later. Hence the comment should be last.
@@ -27,7 +27,7 @@ However, if `left` is *false*, `right` has to be evaluated to determine the outc | |||
|
|||
Another thing to consider when using Boolean operators is their precedence. | |||
```elixir | |||
true or false and false | |||
true or (false and false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a specific point about what this evaluate to just below, maybe those shouldn't be formatted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well spotted
I wrote an Elixir formatter plugin that is able to format Elixir code in Markdown files. @jiegillet @neenjaw what do you think?
(I'll publish it to hex before merging)