Skip to content

Conversation

@milmazz
Copy link
Member

@milmazz milmazz commented May 20, 2015

Changes proposed by @dignifiedquire in his design branch.

@ericmj
Copy link
Member

ericmj commented May 20, 2015

Looks like the execution permission bit is being set for the changed files.

@milmazz
Copy link
Member Author

milmazz commented May 21, 2015

@ericmj Fixed! Thanks for noticing this mistake, definitively I need to review my smb.conf, but that's another issue :)

@josevalim
Copy link
Member

Honestly, I don't want to support fenced code because it is not part of the markdown "spec" (afaik). Unless we are sure all markdown processors (at least the three we support) handle it, I would avoid adding specific behaviour.

@ericmj
Copy link
Member

ericmj commented May 21, 2015

All three markdown engines support fenced code blocks. @milmazz Can you make sure that it's enabled for all of them?

@josevalim
Copy link
Member

Thanks @ericmj.

Then it is also worth debating if we want to starting allowing fenced code blocks in Elixir documentation. I personally find them a hack and unreadable. :(

@ericmj
Copy link
Member

ericmj commented May 21, 2015

I think it's worth the hack since it seems to be the only way to highlight other languages. We don't even have a way to disable the elixir highlighting right now.

@whatyouhide
Copy link
Member

Big 👍 for fenced code blocks, I actually find them extremely more readable than 4-spaces indentation. Also, I'd love to be able to specify the language to highlight; as of now, gettext documentation is full of PO file snippets highlighted as Elixir :(.

@milmazz
Copy link
Member Author

milmazz commented May 21, 2015

@ericmj Perfect, I'll include the fenced code blocks option for the others Markdown parser.

@dignifiedquire
Copy link
Contributor

Adding my vote to allow fenced code blocks. From my personal preference I think they are much more readable, especially when you have indented code inside the code block itself and from an objective standpoint as they are the only way to specify the language.

@josevalim
Copy link
Member

Then we also need to support fenced blocks on IO.ANSI.Docs (which is what elixir uses for formatting markdown inside iex).

@milmazz
Copy link
Member Author

milmazz commented May 21, 2015

I already added the fenced_code_blocks extension for Pandoc. In the case of the Earmark parser this feature is already included through the option gfm

@milmazz
Copy link
Member Author

milmazz commented May 21, 2015

According to this page pandoc supports enable or disable specific syntax extensions by appending them (with + or -) to the writer or reader name (e.g. markdown+fenced_code_blocks) from its version 1.10.

I've looked for more information about the CI environment OS provided by Travis and I found that they use Ubuntu 12.04 LTS Server Edition 64 bit. Also, I found that the version of the pandoc package for Ubuntu 12.04 is 1.9.1.1-1. So, that's why this feature does not work in this environment.

The option that I prefer is proceed to modify the before_install section in the .travis.yml file and then try to include a newer version of the pandoc package. But, I really want to know your opinion on this matter first.

P.S. I've run the tests on Debian 8.0 and everything run smoothly.

@ericmj
Copy link
Member

ericmj commented May 21, 2015

Installing a newer pandoc manually does sound like the best solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ericmj
Copy link
Member

ericmj commented May 27, 2015

@josevalim Are we good to ship this now?
@milmazz Thanks! Please rebase into single commit (commits should be in the present tense and should not end with punctuation).

@josevalim
Copy link
Member

👍

@dignifiedquire
Copy link
Contributor

🎉 🎈

This includes the following:

* Add the `:fenced_code` for the Hoedown parser
* Add the `fenced_code_blocks` extension for Pandoc
* Manually install `pandoc` v1.13.2 via `.travis.yml`

As a side note, in the case of the Earmark parser the support
for fenced code blocks is already included.
@milmazz
Copy link
Member Author

milmazz commented May 27, 2015

@ericmj Please let me know is the latest commit fulfill your suggestions.

@josevalim @ericmj @dignifiedquire @whatyouhide Thanks for your support!

@ericmj
Copy link
Member

ericmj commented May 27, 2015

Perfect! Will merge after CI.

ericmj added a commit that referenced this pull request May 27, 2015
Added the :fenced_code Hoedown option.
@ericmj ericmj merged commit 18d9b9c into elixir-lang:master May 27, 2015
@milmazz milmazz deleted the hoedown_option branch June 3, 2015 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants