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

Use newer `language-` class name prefix #1037

Merged
merged 1 commit into from May 7, 2013
Merged

Use newer `language-` class name prefix #1037

merged 1 commit into from May 7, 2013

Conversation

@djui
Copy link
Contributor

@djui djui commented May 6, 2013

This change makes it partly possible to use Jekyll+RedCarpet+Prism.js without using a plugin.

Another change will respect Jekyll's pygments configuration option and not render the code block using Pygments. Together, these two changes allow using prism.js with Jekyll out of the box.

This change makes it partly possible to use Jekyll+RedCarpet+Prism.js without using a plugin.

Another change will respect Jekyll's `pygments` configuration option and not render the code block using Pygments. Together, these two changes allow using prism.js with Jekyll out of the box.
@mattr-
Copy link
Member

@mattr- mattr- commented May 7, 2013

I don't see a problem with this. 👍 from me.

@caspervonb
Copy link

@caspervonb caspervonb commented May 7, 2013

Neat, was looking to patch this in today myself.

@parkr
Copy link
Member

@parkr parkr commented May 7, 2013

This is only one change. Where's the other?

My biggest concern is that this breaks all the CSS styles people have written for custom code highlighting :/ No backwards-compatibility, unless you set class="lang language-lang".

@djui
Copy link
Contributor Author

@djui djui commented May 7, 2013

The other one is in a separate pull-request ( #1038 ) because they can be seen separate.

I agree with the backwards-compatibility point. Using both class names would be a sufficient solution.

@parkr
Copy link
Member

@parkr parkr commented May 7, 2013

Great! Keep it backwards-compatible, fix the tests and we're good to go.

parkr added a commit that referenced this pull request May 7, 2013
Add newer `language-` class name prefix to code blocks
@parkr parkr merged commit 6b9ef70 into jekyll:master May 7, 2013
1 check failed
1 check failed
default The Travis build failed
Details
parkr added a commit that referenced this pull request May 7, 2013
parkr added a commit that referenced this pull request May 7, 2013
parkr added a commit that referenced this pull request May 7, 2013
@Rowno
Copy link
Contributor

@Rowno Rowno commented May 8, 2013

Any chance you can add a something like a data-lang attribute? I was adding language labels to my code block using the CSS below, but this change breaks it. 😢

code:before {
    content: attr(class);
}
@parkr
Copy link
Member

@parkr parkr commented May 8, 2013

The diff here doesn't reflect the changes entirely. We now use class="<lang> language-<lang>" when writing codeblocks, thus merely adding language-<lang> to the list. Nothing we changed could have ruined the code you have already written, as far as I can tell! :)

@Rowno
Copy link
Contributor

@Rowno Rowno commented May 8, 2013

It ruined it because now I get css language-css etc in my labels (instead of just css), which is why I suggested adding a data-lang attribute.

@parkr
Copy link
Member

@parkr parkr commented May 9, 2013

Gah, sorry, I didn't even really process the code. Late-night brain mode :) I am weary about adding more attributes :/

@Rowno
Copy link
Contributor

@Rowno Rowno commented May 9, 2013

Oh well, it was nice while it lasted. 😞

@parkr
Copy link
Member

@parkr parkr commented May 9, 2013

Tell you what: submit a PR and we'll talk it over with @mattr-. I may be weary, but I'm not opposed. :)

@Rowno
Copy link
Contributor

@Rowno Rowno commented May 9, 2013

Done #1066

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.