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

Switch to Octopress code highlighter #1590

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

imathis
Copy link
Owner

@imathis imathis commented Jun 22, 2014

This PR will replace plugins in the plugins directory with external gems which now offer all the newest code highlighting goodies addressing #1485 and #1472.

@@ -1,43 +1,12 @@
#custom filters for Octopress
require './plugins/backtick_code_block'
require 'jekyll-page-hooks'
#require './plugins/backtick_code_block'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixing soon. Thanks for jumping on this so fast! You're the 💣.

parkr added a commit to parkr/blogsource that referenced this pull request Jun 22, 2014
@parkr
Copy link
Collaborator

parkr commented Jun 22, 2014

Wooohoo! What I have in parkr/blogsource#9 is working for me.

@parkr
Copy link
Collaborator

parkr commented Jun 22, 2014

Just out of curiosity, why are there so many different ways to highlight one's code? Maybe ship with one preferred one and add instructions on how to use the others?

@imathis
Copy link
Owner Author

imathis commented Jun 22, 2014

@parkr That's awesome. It's nice to be able to break things up and externalize them finally. Perhaps the endgame for 2.x is that it has nothing in the plugins directory and just uses gems. That would be lovely.

All I have left on this PR is to replace the include_code plugin. I still have to write it. Perhaps tomorrow. Then I can finally remove the pygments.rb stuff.

Why so many ways? I agree. It's mostly about compatibility. I don't want to work on the codeblock plugin any more since I think it's inferior. I wrote it before I wrote the backtick plugin and honestly I have no idea if people are event using it. Thankfully because of the work I've done with octopress-code-highlighter it took me about 20 minutes to write octopress-codeblock. I did it because it will let me externalize the plugin and remove the cruft without disrupting people's blogs when they update.

It's really the only plugin that has a duplicate function. Gist, include_code, and codefence aren't ways of doing the same thing.

@parkr
Copy link
Collaborator

parkr commented Jun 22, 2014

Perhaps the endgame for 2.x is that it has nothing in the plugins directory and just uses gems. That would be lovely.

That would be lovely! 😃

Why so many ways? I agree. It's mostly about compatibility.

Fair enough. I was just thinking about the overlap and was concerned but it's a MAJOR bump change to remove the duplication, not a MINOR change so it'll be for 3.0 instead of 2.5 or whatever this release ends up being. 😄

I did it because it will let me externalize the plugin and remove the cruft without disrupting people's blogs when they update.

You never fail to think of the user first. An exceptional quality in a software developer – tantamount to a virtue in engineering – and something most overlook. You rock! 🤘

@imathis
Copy link
Owner Author

imathis commented Jun 23, 2014

With this update, Octopress's code stuffs will finally match the docs. 🍧

I shipped three new gems to make it happen. Feels good.

@parkr
Copy link
Collaborator

parkr commented Jun 23, 2014

I shipped three new gems to make it happen. Feels good.

Fleet Admiral up in here! 🚢 🚢 🚢 🚢 🚢

gem 'octopress-codefence', '~> 1.4.2'
gem 'octopress-codeblock', '~> 1.0.1'
gem 'octopress-gist', '~> 1.3.0'
gem 'octopress-render-code', '~> 1.0.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason you're being so specific here? Why specify the PATCH levels?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Good call, I should broaden these.

parkr added a commit to parkr/blogsource that referenced this pull request Jun 23, 2014
@parkr
Copy link
Collaborator

parkr commented Jun 23, 2014

Hrm, I set $solarized: dark in my sass/_base.scss but get this tiny, tiny difference:

screen shot 2014-06-23 at 1 24 13 pm

Looks like it's just that initial $ (belongs to the .gp class). Is that no longer part of the Sass highlighting? It's fine, I just noticed the difference and wanted to ask 😄

And yes, I use Opera. It's the best.

@imathis
Copy link
Owner Author

imathis commented Jun 23, 2014

Good catch on the code style comparison. I've activated more solarized colors now and addressed the loosening the Gem version specificity.

@parkr
Copy link
Collaborator

parkr commented Jun 23, 2014

I've activated more solarized colors now and addressed the loosening the Gem version specificity.

💥 ! I think this guy is ready to 🚢, then. What do you think?

@imathis
Copy link
Owner Author

imathis commented Jun 23, 2014

Ok I think this is all working. I probably need to do a blog post before merging since people will need to either update their classic theme, or theme makers will need to update their stylesheets to work with the new html output from the code-highlighter.

@imathis
Copy link
Owner Author

imathis commented Jun 26, 2014

I think all I have left to do is add a style for plain <code> elements to the code sass.

@parkr
Copy link
Collaborator

parkr commented Jun 26, 2014

I think all I have left to do is add a style for plain <code> elements to the code sass.

👏 🐰

@ramsey
Copy link

ramsey commented Jun 27, 2014

Correct. I don't have to specify that for GitHub flavored markdown, and it's the desirable default—for me, at least. :-)

@imathis
Copy link
Owner Author

imathis commented Jun 27, 2014

@ramsey I've done a bit of testing. If I set the default to be on, it doesn't error or anything. However, with startinline:true, if you are trying to do a typical PHP embedded in HTML example, it highlights the HTML as well. I'm not sure if that is a big deal or not.

The top example is with startinline:true the bottom is the standard PHP lexer.

image

What do you think?

@imathis
Copy link
Owner Author

imathis commented Jun 27, 2014

@parkr Do you have any knowledge of how GitHub handles this on pages builds? If I'm using GitHub pages, is this option set to true in pygments?

@ramsey
Copy link

ramsey commented Jun 27, 2014

That doesn't bother me. Of course, I can't speak for everyone, but there is also the html+php lexer, if folks want to use that for proper HTML & PHP highlighting: http://pygments.org/docs/lexers/#pygments.lexers.templates.HtmlPhpLexer

@imathis
Copy link
Owner Author

imathis commented Jun 27, 2014

I'll definitely enable the option, I'm not sure if I should default it to true yet though. Also here's one thing that bugs me. No matter what it seems that the PHP lexer fails to highlight the first thing, be it an HTML tag or a variable name.

image

Oddly enough though, it seems to highlight just fine here on GitHub. I wish I knew what they were doing differently.

@parkr
Copy link
Collaborator

parkr commented Jun 27, 2014

@imathis I don't have any insights into how GitHub comments are rendered, but I don't think they use Pygments. They use the github-markdown gem, perhaps?

@imathis
Copy link
Owner Author

imathis commented Jun 27, 2014

@parkr, it looks like GitHub uses Linguist which highlights with Pygments.rb. So somehow it's different but I can't see how.

@imathis
Copy link
Owner Author

imathis commented Jun 27, 2014

Alright, so with the merge to the code-highlighter project, this is all good to go. The changelog is a pretty conservative view of what has happened since 2.0, but I don't want to have to figure out intermediary steps between now and 2.0. This project has been a failure in terms of semantic version, so I think it's probably easiest to set this as 2.1 and promise to do better in the future.

I'm thinking I'll do a blog post before merging this into master and try to get third-party theme creators to make the necessary updates to their themes to support the new code styles. Also I'll tease some of the stuff planned for the 3.0 release.

@parkr
Copy link
Collaborator

parkr commented Jun 27, 2014

it looks like GitHub uses Linguist which highlights with Pygments.rb. So somehow it's different but I can't see how.

Linguist is used for files (blobs) but not for comments. They used to use Github-Flavoured Markdown but switched to HTML Pipeline stuff, I think.

[everything else]

👍 Woot! If you want any help writing/editing that blog post, I'm happy to help!

stevenharman added a commit to stevenharman/stevenharman.net-octopress that referenced this pull request Jul 10, 2014
@garfieldnate
Copy link

👍 excited to pull this.

parkr added a commit to parkr/blogsource that referenced this pull request Jul 17, 2014
@parkr
Copy link
Collaborator

parkr commented Jul 17, 2014

@imathis Looks like there's a small bug:

  Liquid Exception: uninitialized constant Jekyll::PullquoteTag::RubyPants in _posts/2013-04-30-an-afterlife.markdown
/Users/parker/code/blog/plugins/pullquote.rb:36:in `render': uninitialized constant Jekyll::PullquoteTag::RubyPants (NameError)

@parkr
Copy link
Collaborator

parkr commented Jul 17, 2014

@imathis Also, there's a weird change with inline backticks:

screen shot 2014-07-16 at 10 26 00 pm

They used to be wrapped in a nice little pocket:

screen shot 2014-07-16 at 10 27 34 pm

@imathis
Copy link
Owner Author

imathis commented Jul 17, 2014

@parkr
Copy link
Collaborator

parkr commented Jul 17, 2014

@imathis Interesting. I did this, and received a slightly different result:

screen shot 2014-07-17 at 2 08 00 am

vs

screen shot 2014-07-17 at 2 08 49 am

I personally prefer the second option. What do you think?

@imathis
Copy link
Owner Author

imathis commented Jul 17, 2014

The first option is nearly a ripoff of what GitHub does for their inline code blocks. I can brighten the background though, I think it'd look better.

@parkr
Copy link
Collaborator

parkr commented Jul 17, 2014

I guess it's just what I'm used to seeing on my blog.

I can brighten the background though, I think it'd look better.

I think that would make them pop better! I like it when they're a bit more intrusive – they're supposed to indicate a flip in the perception of the reader: not just prose, but contextual information about the program or procedure I'm describing.

@mbrgm
Copy link

mbrgm commented Nov 13, 2014

Is there any progress on this PR? I'd love to see that feature in master.

@jmhwang7
Copy link

Wondering the same as mbrgm. Is there any progress on this?

@ramsey
Copy link

ramsey commented Mar 12, 2015

I've been using Octopress 3 with these migration instructions to use the new octopress-codeblock, among other things. It's working out quite nicely.

@jmhwang7
Copy link

Awesome! Thanks! I'll try that. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants