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

Replace backticks within HTML blocks with HTML tags #5435

Merged
merged 6 commits into from Nov 20, 2016

Conversation

Projects
None yet
9 participants
@alexmalik
Contributor

alexmalik commented Oct 1, 2016

Prior to the change backticks were used in an attempt to create a
code block. The problem is that inside block level HTML tags Markdown
is not supported. I have replaced the backticks with a combination of
HTML tags in order to approximately simulate the appearance of a code
block. The docs suggest possible use of span tags in place of the
surrounding div tags as a solution to getting the Markdown to render.
I tried this but no success.

This change improves the readers understanding of the information,
because the reader doesn't have to make sense of raw markdown.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Oct 1, 2016

Member

This seems to make things more difficult to read. Why is the code block broken up between multiple <p>?

I don't think I understand the problem being fixed here.

Member

pathawks commented Oct 1, 2016

This seems to make things more difficult to read. Why is the code block broken up between multiple <p>?

I don't think I understand the problem being fixed here.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 2, 2016

Member

A screenshot helps in putting the point forward..

Issue Screengrab

backticks

Member

ashmaroli commented Oct 2, 2016

A screenshot helps in putting the point forward..

Issue Screengrab

backticks

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 2, 2016

Member

I'm not in favour of using multiple <p></p> tags here. Please simulate a generated code block by using <pre class="highlight"><code>[code-block]</code></pre> Its not entirely correct either.. you wont get the lines to highlight, but you can simulate the code block at a fine extent.

Pro-Tip: Don't forget to preview your changes to site before pushing.
You can do so by running bundle exec rake site:preview

/cc @jekyll/documentation

Member

ashmaroli commented Oct 2, 2016

I'm not in favour of using multiple <p></p> tags here. Please simulate a generated code block by using <pre class="highlight"><code>[code-block]</code></pre> Its not entirely correct either.. you wont get the lines to highlight, but you can simulate the code block at a fine extent.

Pro-Tip: Don't forget to preview your changes to site before pushing.
You can do so by running bundle exec rake site:preview

/cc @jekyll/documentation

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 2, 2016

Member

Also, I'd suggest the title to be edited to :
Replace backticks within HTML blocks with HTML tags

Member

ashmaroli commented Oct 2, 2016

Also, I'd suggest the title to be edited to :
Replace backticks within HTML blocks with HTML tags

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Oct 2, 2016

Member

@ashmaroli Thank you; that is the part I was missing.

Could we use the {% highlight %} tag?

Member

pathawks commented Oct 2, 2016

@ashmaroli Thank you; that is the part I was missing.

Could we use the {% highlight %} tag?

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 2, 2016

Member

@pathawks, you're welcome. :)
I'd tried {% highlight ruby %} the resulting code block highlighted the ruby syntax correctly, but the overall styling was a bit out of place.. Maybe I could have got it to work if I played around a bit.. but I decided to let the contributor discover that by himself..

Member

ashmaroli commented Oct 2, 2016

@pathawks, you're welcome. :)
I'd tried {% highlight ruby %} the resulting code block highlighted the ruby syntax correctly, but the overall styling was a bit out of place.. Maybe I could have got it to work if I played around a bit.. but I decided to let the contributor discover that by himself..

@alexmalik alexmalik changed the title from Replace backticks with HTML tags to Replace backticks within HTML blocks with HTML tags Oct 2, 2016

@alexmalik

This comment has been minimized.

Show comment
Hide comment
@alexmalik

alexmalik Oct 2, 2016

Contributor

Thanks for your feedback on the PR everyone :)
@ashmaroli thank you especially for the tips!

Contributor

alexmalik commented Oct 2, 2016

Thanks for your feedback on the PR everyone :)
@ashmaroli thank you especially for the tips!

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Oct 2, 2016

Contributor

We no longer use {% highlight %} in Jekyll's site source, it was optioned and voted to start using triple backticks, this was done not too long ago, so unless you absolutely need highlight, you should properly space out your backticks and use those.

Contributor

envygeeks commented Oct 2, 2016

We no longer use {% highlight %} in Jekyll's site source, it was optioned and voted to start using triple backticks, this was done not too long ago, so unless you absolutely need highlight, you should properly space out your backticks and use those.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 2, 2016

Member

so unless you absolutely need highlight,

@envygeeks, this is one such instance where I think we absolutely need to use highlight. I authored the commit that replaced {% highlight %} with backticks. What I didnt know till now, was that backticks dont seem to get picked up as expected when they are inside an HTML element.

Member

ashmaroli commented Oct 2, 2016

so unless you absolutely need highlight,

@envygeeks, this is one such instance where I think we absolutely need to use highlight. I authored the commit that replaced {% highlight %} with backticks. What I didnt know till now, was that backticks dont seem to get picked up as expected when they are inside an HTML element.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 2, 2016

Member

@alexmalik, I tested your branch locally. This is the problem with using {% highlight %}.

gupg

The best solution would hence be to manually code the blocks in HTML.
few tips:

  • HTML generated by backticks is slightly different from that generated by highlight
  • The former gives you a div with class="highlighter-rouge", while that latter gives you a figure with class="highlight"
  • You dont have to manually code the syntax highlight spans. It's already available in your branch inside _site/docs/github-pages/index.html
  • The same page has another block generated by backticks above the current scene of action. That'll tell you how your own code should look like.
Member

ashmaroli commented Oct 2, 2016

@alexmalik, I tested your branch locally. This is the problem with using {% highlight %}.

gupg

The best solution would hence be to manually code the blocks in HTML.
few tips:

  • HTML generated by backticks is slightly different from that generated by highlight
  • The former gives you a div with class="highlighter-rouge", while that latter gives you a figure with class="highlight"
  • You dont have to manually code the syntax highlight spans. It's already available in your branch inside _site/docs/github-pages/index.html
  • The same page has another block generated by backticks above the current scene of action. That'll tell you how your own code should look like.
@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Oct 3, 2016

Member

This is the problem with using {% highlight %}.

Looks like it's just a problem with CSS, not with the tag itself.

Member

pathawks commented Oct 3, 2016

This is the problem with using {% highlight %}.

Looks like it's just a problem with CSS, not with the tag itself.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 3, 2016

Member

@pathawks, like I mentioned above, the markup differs too:
{% highlight ruby %} generates :

<figure class="highlight">
  <pre>
    <code class="language-ruby" data-lang="ruby">
      <span></span>
    </code>
  </pre>
</figure>

while backticks ruby generates

<div class="language-ruby highlighter-rouge">
  <pre class="highlight">
    <code>
      <span></span>
    </code>
  </pre>
</div>
Member

ashmaroli commented Oct 3, 2016

@pathawks, like I mentioned above, the markup differs too:
{% highlight ruby %} generates :

<figure class="highlight">
  <pre>
    <code class="language-ruby" data-lang="ruby">
      <span></span>
    </code>
  </pre>
</figure>

while backticks ruby generates

<div class="language-ruby highlighter-rouge">
  <pre class="highlight">
    <code>
      <span></span>
    </code>
  </pre>
</div>
@mmistakes

This comment has been minimized.

Show comment
Hide comment
@mmistakes

mmistakes Oct 3, 2016

@ashmaroli Kramdown has a mechanism for parsing Markdown inside of HTML. If you wrap it with something like a <div> element and give it an attribute of markdown="1" it'll process triple backticks (or any other Markdown).

<div markdown="1">
  triple backtick code bloc
</div>

output

Still doesn't fix the CSS that will likely need to be adjusted for .note, but at least you don't have to manually simulate Rouge's output.

mmistakes commented Oct 3, 2016

@ashmaroli Kramdown has a mechanism for parsing Markdown inside of HTML. If you wrap it with something like a <div> element and give it an attribute of markdown="1" it'll process triple backticks (or any other Markdown).

<div markdown="1">
  triple backtick code bloc
</div>

output

Still doesn't fix the CSS that will likely need to be adjusted for .note, but at least you don't have to manually simulate Rouge's output.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 3, 2016

Member

niceee.. 👍
Thank you @mmistakes 😃

Member

ashmaroli commented Oct 3, 2016

niceee.. 👍
Thank you @mmistakes 😃

@parkr parkr added the documentation label Oct 3, 2016

ashmaroli added a commit to ashmaroli/jekyll that referenced this pull request Oct 4, 2016

use `markdown="1"` attribute with HTML tags
the use of this attribute allows kramdown to parse markdown within HTML
elements. More info can be found here:
http://kramdown.gettalong.org/syntax.html#html-blocks

Thanks:
 - @alexmalik (https://github.com/alexmalik)
   for starting the pull request at
   jekyll#5435
   which highlighted the issue and led to this.

 - Michael Rose (https://github.com/mmistakes)
   for bringing this attribute to my attention.

ashmaroli added a commit to ashmaroli/jekyll that referenced this pull request Oct 4, 2016

use `markdown="1"` attribute with HTML tags
the use of this attribute allows kramdown to parse markdown within HTML
elements. More info can be found here:
http://kramdown.gettalong.org/syntax.html#html-blocks

Thanks:
 - @alexmalik
   for starting the pull request at jekyll#5435
   which highlighted the issue and led to this.

 - Michael Rose (@mmistakes)
   for bringing this attribute to my attention.
@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 4, 2016

Member

It looks like GFM doesnt work like kramdown in this regard too..
GFM seems to require an empty closed <element> with markdown="1" to be present for every nested html element within the concerned element..

Confused?
Check this branch compare on my fork to understand.

@alexmalik : you're welcome to cherry-pick the commits on that branch to yours, or manually commit them to your branch, whichever way you prefer to, for review here.

Member

ashmaroli commented Oct 4, 2016

It looks like GFM doesnt work like kramdown in this regard too..
GFM seems to require an empty closed <element> with markdown="1" to be present for every nested html element within the concerned element..

Confused?
Check this branch compare on my fork to understand.

@alexmalik : you're welcome to cherry-pick the commits on that branch to yours, or manually commit them to your branch, whichever way you prefer to, for review here.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 4, 2016

Member

@alexmalik, the reason we decided to use backticks over {% highlight %} is because by using backticks, markdown pages can be viewed as intended, even at the repo instead of only at jekyllrb.com
If you VIEW the github-pages.md in your branch, you'll see that the section under consideration does not have the fenced-code block rendered properly. Hence you need to implement the second commit in my branch-compare as well.

Member

ashmaroli commented Oct 4, 2016

@alexmalik, the reason we decided to use backticks over {% highlight %} is because by using backticks, markdown pages can be viewed as intended, even at the repo instead of only at jekyllrb.com
If you VIEW the github-pages.md in your branch, you'll see that the section under consideration does not have the fenced-code block rendered properly. Hence you need to implement the second commit in my branch-compare as well.

@alexmalik

This comment has been minimized.

Show comment
Hide comment
@alexmalik

alexmalik Oct 4, 2016

Contributor

@ashmaroli it looks ok to me, the slightly darker background of the code blocks is removed, improving the uniformity. Is this ok?

screen shot 2016-10-04 at 12 36 27

Contributor

alexmalik commented Oct 4, 2016

@ashmaroli it looks ok to me, the slightly darker background of the code blocks is removed, improving the uniformity. Is this ok?

screen shot 2016-10-04 at 12 36 27

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Oct 4, 2016

Member

@alexmalik, the page at jekyllrb looks 👍
but look how it is at Github..

the page on your branch..

a

the page on mine..

b

Member

ashmaroli commented Oct 4, 2016

@alexmalik, the page at jekyllrb looks 👍
but look how it is at Github..

the page on your branch..

a

the page on mine..

b

@ashmaroli

now its proper 👍

@pathawks

Thank you so much for working on this.

My only concern is that I would like to see Markdown style links ([]()) used instead of HTML links (<a href="">)

Everything else looks good to me 👍

Show outdated Hide outdated site/_docs/github-pages.md
##### Use the `github-pages` gem
Our friends at GitHub have provided the
<a href="https://github.com/github/pages-gem">github-pages</a>

This comment has been minimized.

@pathawks

pathawks Oct 21, 2016

Member

Can we make this a Markdown style link?

[github-pages](https://github.com/github/pages-gem)
@pathawks

pathawks Oct 21, 2016

Member

Can we make this a Markdown style link?

[github-pages](https://github.com/github/pages-gem)
Show outdated Hide outdated site/_docs/github-pages.md
If you like to install <code>pages-gem</code> on Windows you can find instructions by Jens Willmer on <a href="http://jwillmer.de/blog/tutorial/how-to-install-jekyll-and-pages-gem-on-windows-10-x46#github-pages-and-plugins">how to install github-pages gem on Windows (x64)</a>.
</p>
If you like to install `pages-gem` on Windows you can find instructions by Jens Willmer on <a href="http://jwillmer.de/blog/tutorial/how-to-install-jekyll-and-pages-gem-on-windows-10-x46#github-pages-and-plugins">how to install github-pages gem on Windows (x64)</a>.

This comment has been minimized.

@pathawks

pathawks Oct 21, 2016

Member

Can we use a Markdown style link here as well?

@pathawks

pathawks Oct 21, 2016

Member

Can we use a Markdown style link here as well?

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Nov 9, 2016

Member

hey @alexmalik sorry for not merging this sooner, could you rebase this please now that the docs have moved to /docs/_docs/?

Member

DirtyF commented Nov 9, 2016

hey @alexmalik sorry for not merging this sooner, could you rebase this please now that the docs have moved to /docs/_docs/?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Nov 20, 2016

Contributor

I'll 👍 after a rebase!

Contributor

envygeeks commented Nov 20, 2016

I'll 👍 after a rebase!

alexmalik added some commits Oct 1, 2016

Replace backticks with HTML tags
Prior to the change backticks were used in an attempt to create a
code block. The problem is that inside block level HTML tags Markdown
is not supported. I have replaced the backticks with a combination of
HTML tags in order to approximately simulate the appearance of a code
block. The docs suggest possible use of span tags in place of the
surrounding div tags as a solution to getting the Markdown to render.
I tried this but no success.

This change improves the readers understanding of the information,
because the reader doesn't have to make sense of raw markdown.
replace {% %} with bacticks for nested code-block
uses Kramdown with the markdown="1" attribute, as suggested by
@mmistakes. This allows rendering of code blocks which are nested
inside HTML tags.
add empty div with markdown="1" attribute
an empty div is necessary in order for the code blocks to render
correctly when not displayed on the jekyllrb site.
@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Nov 20, 2016

Member

Rebased 1b3cf68 onto master

Member

pathawks commented Nov 20, 2016

Rebased 1b3cf68 onto master

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Nov 20, 2016

Member

Thanks everyone for helping fix this rendering issue.

@jekyllbot: merge +site

Member

DirtyF commented Nov 20, 2016

Thanks everyone for helping fix this rendering issue.

@jekyllbot: merge +site

@DirtyF DirtyF closed this Nov 20, 2016

@jekyllbot jekyllbot merged commit 1936c21 into jekyll:master Nov 20, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

jekyllbot added a commit that referenced this pull request Nov 20, 2016

@alexmalik alexmalik deleted the alexmalik:github-pages-backticks branch Nov 21, 2016

@alexmalik

This comment has been minimized.

Show comment
Hide comment
@alexmalik

alexmalik Nov 21, 2016

Contributor

Thanks for your help everyone 👍

Contributor

alexmalik commented Nov 21, 2016

Thanks for your help everyone 👍

@djacquel

This comment has been minimized.

Show comment
Hide comment
@djacquel

djacquel Jan 27, 2017

Setting a :

kramdown:
  parse_block_html: true

was enough for me to fix this.

djacquel commented Jan 27, 2017

Setting a :

kramdown:
  parse_block_html: true

was enough for me to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment