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

No highlighting option? #344

Closed
GuiTeK opened this issue Aug 23, 2014 · 26 comments
Closed

No highlighting option? #344

GuiTeK opened this issue Aug 23, 2014 · 26 comments

Comments

@GuiTeK
Copy link

GuiTeK commented Aug 23, 2014

I'd like to create a code block with some Apache configuration, however, there is no such language definition for Prism at the moment.

I'd be happy with just a code block with plain black text and no syntax highlighting but I couldn't find a way to do that.

  • Does such a feature exist?
  • If not, should it be added?
  • Or would it be preferred to write the definition for the Apache conf syntax?
@LeaVerou
Copy link
Member

Hey there,
You can read how to add new languages here: http://prismjs.com/extending.html
If you know regexes, it’s really easy!

@GuiTeK
Copy link
Author

GuiTeK commented Aug 24, 2014

I guess I have to do it then 😛 .

Here is the JavaScript part: see below comment https://github.com/GuiTeK/prism/commit/98b5c0f9b45309879188be8f5de80df07a91f027
I didn't do a PR yet. Tell me if it seems correct.

Also, I was wondering where I should add the CSS part (I created three classes):

  • In themes/prism.js
  • In every themes/prism-theme.css

?

EDIT: preview screenshot

@GuiTeK
Copy link
Author

GuiTeK commented Aug 24, 2014

I've made some fixes and additions.
See here: https://github.com/GuiTeK/prism/commit/ed6c0663cdfd7f7992cbc116d826f26b06b72c03

@apfelbox
Copy link
Contributor

If you need to add new tokens, you should add them to every themes themes/prism-....css (so that your language has proper support in every theme).

@GuiTeK
Copy link
Author

GuiTeK commented Aug 25, 2014

@apfelbox Thank you.
I sent a PR.

@LeaVerou
Copy link
Member

However, it should be noted that you should try to be conservative about the tokens you add.

@GuiTeK
Copy link
Author

GuiTeK commented Aug 25, 2014

There should be no need to add other tokens.
I could have used existing tokens but the class names wouldn't have reflected the reality. Tell me if that's a problem and I should use existing tokens.

@LeaVerou
Copy link
Member

It’s a tradeoff. If it’s possible to use existing tokens it’s preferred, but if existing tokens are way too far from the semantic meaning of the tokens in your language, it’s better to add new ones.

@GuiTeK
Copy link
Author

GuiTeK commented Aug 25, 2014

I could use the existings .token.property and .token.tag respectively for .token.directive-inline and .token.directive-block but it's quite far from the original meaning.
Then I could use .token.attr-value and .token.keyword respectively for .token.directive-block-parameter and .token.directive-flags but once again that does not make much sense.

What's your opinion?

@apfelbox
Copy link
Contributor

@LeaVerou couldn't we use #328 for this?
So we could define the actual token types (.directive-block) and alias them to existing token subtypes (.tag or something).

So we have a tradeoff between semantic tags and reusing the existing ones?

@zeitgeist87
Copy link
Collaborator

@apfelbox alias would be a great name for the option ...

@apfelbox
Copy link
Contributor

@zeitgeist87 you are right, lol. 😎

@GuiTeK
Copy link
Author

GuiTeK commented Aug 25, 2014

Indeed #328 might be the solution.
This way there is no need to modify the themes for a single language definition and it allows for relevant semantic tags.

@GuiTeK
Copy link
Author

GuiTeK commented Aug 29, 2014

Even if I opened this issue for the Apache configuration syntax and this is solved, I still believe a language-none would be useful.

Indeed:

  • there are lots of syntaxes which are not supported by Prism (not especially by Prism, by most of syntax highlighters), for good reason: we can't add them all. Some softwares use their own syntax for their configuration files and we can't add each and every custom syntax into Prism.
  • sometimes you just want to show some program output in a code block, which is not a particular syntax

As Prism doesn't format <pre> tags if no valid language is specified, I think a language-none would be useful.

@LeaVerou
Copy link
Member

language-none is a hack, it shouldn’t needed. If it is, that’s a bug with Prism.

@brandones
Copy link

For those finding this later -- Prism now formats <pre> tags even if no valid language is specified!

@jfcere
Copy link

jfcere commented Apr 5, 2019

@brandones as per v1.16 having <pre><code>I am only text</code></pre> without specyfing any language-x class does not seems to be displayed as a code block.

Is it intended or did I misinterpreted your last comment?

image

@RunDevelopment
Copy link
Member

[...] even if no valid language is specified!

@jfcere I think Prism used to not work when the specified language was unknown to Prism.
So, no. We don't support <pre><code> without a language-xxxx class for at least one of the elements.

@jfcere
Copy link

jfcere commented Apr 5, 2019

@RunDevelopment thanks for your follow up.

Is this something you guys would be willing to do? I could do the PR if you guys agree.

Although I could be wrong, I think it is pretty much expected that <pre><code> are displayed as code block even if there is no language-xxxx. The only reason I could think for not doing it is that it could be interpreted as too intrusive.

Anyhow, I'll respect your decision and will find my way around this if the proposition is rejected.

@mAAdhaTTah
Copy link
Member

The underlying issue is most themes do something like this to style pre & code elements. I do think it would be too intrusive to apply styles to every pre & code because we can't assume all of them will be touched by Prism (in fact, if it doesn't have a language Prism core will ignore them). Seems like the themes should follow suit, so I'm inclined to reject that suggestion.

This also would technically be a breaking change, as we're now styling things we weren't before, and would require consumers to overwrite our styles if they didn't want them.

@RunDevelopment
Copy link
Member

@jfcere I think that this would create the problem that you can't opt out anymore.

Right now, Prism is strictly opt-in meaning that only the code blocks you give a special language-xxxx class are affected by Prism.
So if we change this, we should also include an opt-out option.

I don't know if this is worth the hassle, plus it would be a breaking change as @mAAdhaTTah pointed out while I wrote this.

Also, Prism supports the inheritance of language-xxxx classes. So you could just give your <body> the class language-none and every block will be highlighted which means that plugins like e.g. line number and CSS styles will also be applied.
So, do we even need to change anything?

@jfcere
Copy link

jfcere commented Apr 5, 2019

Alright, I understand and it makes a lot of sense.

My situation is different because I am using Prism inside Angular framework which only applies Prism on specified elements instead of the whole page making the concern of not wanting the code to be highlighted for a certain element irrelevant in that context (you can look my repository ngx-markdown if you are curious).

Thanks again for your follow up and this awesome library!

@RunDevelopment
Copy link
Member

In that case, I assume that it is not possible to call the Prism.highlightElement function yourself?

It will behave a little strangely, but it should work. (If I read the code correctly, you'll get nice little language-undefined class.)

On that note: Maybe we should make it so that language-none is the default in the case that highlightElement can't detect a language-xxxx class instead of language- or language-undefined @mAAdhaTTah ?

@mAAdhaTTah
Copy link
Member

You could wire a customer renderer to change inline code blocks to have the expected class.

@LeaVerou
Copy link
Member

LeaVerou commented Apr 5, 2019

FWIW, I agree with the others about not targeting class-less <pre>s and <code>s. Do note that since language classes inherit, targeting all of them can be as simple as applying language-none to your <html>, <body> or any other shared ancestor.

On that note: Maybe we should make it so that language-none is the default in the case that highlightElement can't detect a language-xxxx class instead of language- or language-undefined

Yeah, that sounds like a nice little fix, so that code listings with no language can be styled with one class. I wonder if there's any value in distinguishing the two cases though.

@RunDevelopment
Copy link
Member

I wonder if there's any value in distinguishing the two cases though.

I don't think there is.
TBH I don't even think that the language- case is possible when looking at the code. I can't imagine a case where parent.className.match(lang) returns null.

scottopherson pushed a commit to chef/automate that referenced this issue Apr 23, 2020
`chef-snippet` will throw an error if the syntax grammar rules for the language are unavailable.

Instead, this commit modifies the code highlighter to fallback to rendering code blocks as uncolored plain text in this scenario.
PrismJS/prism#344 (comment)

Fixes #3462

Signed-off-by: Scott Christopherson <[email protected]>
scottopherson pushed a commit to chef/automate that referenced this issue Apr 23, 2020
`chef-snippet` will throw an error if the syntax grammar rules for the language are unavailable.

Instead, this commit modifies the code highlighter to fallback to rendering code blocks as uncolored plain text in this scenario.
PrismJS/prism#344 (comment)

Fixes #3462

Signed-off-by: Scott Christopherson <[email protected]>
susanev pushed a commit to chef/automate that referenced this issue Apr 24, 2020
`chef-snippet` will throw an error if the syntax grammar rules for the language are unavailable.

Instead, this commit modifies the code highlighter to fallback to rendering code blocks as uncolored plain text in this scenario.
PrismJS/prism#344 (comment)

Fixes #3462

Signed-off-by: Scott Christopherson <[email protected]>
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

No branches or pull requests

9 participants