Skip to content

Conversation

@joshgoebel
Copy link
Member

Closes #2330.

- Adds a separate language to detect PHP templates to avoid overloading
  `xml`

XML never should have built-in PHP support in the first place as there
are any number of templating languages out there, and they all can't be
built into XML.

This rectifies that situation.
@joshgoebel joshgoebel added enhancement An enhancement or new feature language labels Feb 20, 2020
export default function(hljs) {
return {
name: "PHP template",
subLanguage: 'xml',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I can't say I like this two-grammars approach very much...
But looking at your PR I got a thought: what if we allow two sublanguages here? I. e.

    subLanguage: ['xml','php'],

This way we got one grammar that resolves into either xml + contained php, or just php.
If only we had a way to disallow php to act as a top-level language...

Copy link
Member Author

Choose a reason for hiding this comment

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

If only we had a way to disallow php to act as a top-level language...

But it IS a top-level language... someone could post a snippet of PHP without the tags... in which case it's a php snippet, ie php code... meanwhile a php-template is HTML+PHP...

what if we allow two sublanguages here?

That makes me head hurt - and what is the advantage of doing it that way? PHP code on it's own is NOT a php template.

Copy link
Member Author

@joshgoebel joshgoebel Feb 25, 2020

Choose a reason for hiding this comment

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

I can't say I like this two-grammars approach very much...

It's consistent with how we do VBScript-HTML and how I've suggested we do other templating languages. For these type of things you have 3 things happening:

  • The core language (say Ruby)
  • The tagging/templating language (say ERB)
  • The actual output format (say HTML)

Those would be 3 different grammars. This is true for PHP also... the language itself is technically distinct from the templating language.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just rename php to _php, and php-template to php.
So that you can use single php for both php snippets and php templates.

Copy link
Member Author

@joshgoebel joshgoebel Feb 25, 2020

Choose a reason for hiding this comment

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

How would that work for ERB or VBscript? I'm trying to also look at the BIG picture here on how we handling templating in general - we should try and be consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

So that you can use single php for both php snippets and php templates.

REMEMBER, no one is using php now for PHP... well they might be, but they're more likely using xml since that highlights HTML + PHP.

Copy link
Collaborator

Choose a reason for hiding this comment

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

REMEMBER, no one is using php now for PHP... well they might be, but they're more likely using xml since that highlights HTML + PHP.

Right. And we consider it bad, because it makes xml grammar bloated.
On the other hand, adding xml support to the core language doesn't seem bad. Or it does?

How would that work for ERB or VBscript?

The same way.
(leaving aside existing ERB and VBScript-html for the sake of discussion)

@joshgoebel
Copy link
Member Author

joshgoebel commented Feb 25, 2020

subLanguage: ['xml','php'], 

That doesn't work because that's EITHER OR. Either it will be colored HTML OR it will be colored PHP. That's only 2 out of the 3, hence why we're adding php-template.

  • PHP code
  • HTML code
  • PHP template (mixed HTML and PHP) [this case isn't covered]

Note: That might have worked in the past, but it is NOT correct for php (or any templating languages) to be hidden inside xml itself... so that means XML is purely XML/HTML.

@joshgoebel
Copy link
Member Author

I don't think we have a perfect way of doing this currently (see the longer issue thread). I personally think php and php-template is better than PHP and XML. And in the future if we add features to allow us to fix this then we could always just make php-template an alias of the new super powered php.

@egor-rogov
Copy link
Collaborator

That doesn't work because that's EITHER OR. Either it will be colored HTML OR it will be colored PHP. That's only 2 out of the 3, hence why we're adding php-template.

Why? This part takes care of it:

    contains: [
      {
        begin: /<\?(php|=)?/,
        end: /\?>/,
        subLanguage: 'php',
        ...

I personally think php and php-template is better than PHP and XML.

Do you mean "is better than existing PHP and XML"?
What do you think of (in principle, no matter how technically implemented) having php for both php code and php templates, and xml for XML/HTML? And likewise, ruby for both ruby and erb, etc.

@joshgoebel
Copy link
Member Author

joshgoebel commented Feb 25, 2020

Why? This part takes care of it:

I'm not sure I follow... How? Where is that code snippet? As previously mentioned you can't put every single templating language inside XML - and it's inconsistent. We already have many HTML templating languages (Twig, handlebars, HTMLBars, etc)... they are always their own grammar.

When you do something like sublanguage: [php, html] That means ONE will be picked. So if you're doing that at the top-level then you're really just picking php or html, but that does not get you php + html.

So really what you're suggesting is PHP = `['_php', '_php-template']...

Do you mean "is better than existing PHP and XML"?

Yes.

What do you think of (in principle, no matter how technically implemented) having php for both php code and php templates, and xml for XML/HTML?

As a general way of thinking about it I'm not opposed. But I think this is specific to how PHP is thought about. PHP has historically been defined as "PHP/HTML"...

And likewise, ruby for both ruby and erb, etc.

Very much disagree here. Ruby and ERB are not interchangeable. Ruby code is Ruby code and ERB is a templating language based on Ruby. They are discrete things and thought of as such.

@joshgoebel
Copy link
Member Author

So really what you're suggesting is PHP = `['_php', '_php-template']...

I'd have to give this some more thought, but it produces really ugly HTML:

<div class="hljs php">
<div class="_php-template"><div class="xml">...</div>
<div class="_php>...</div>
<div class="xml">...</div>
<div class="_php>...</div>
<div class="xml">...</div>
</div>
...

@joshgoebel
Copy link
Member Author

joshgoebel commented Feb 25, 2020

This is going to a lot of trouble just to build a "nice php alias"... Since that's the ONLY additional thing doing it the way you suggest gives you. Having "php" and "php-template" for auto-detect works just fine, auto-detect will decide... but if you want the user to only ever have to think php... then you end up going down a more crazy path.

I'd like to suggest that php and php-template are clearer than php and xml. And it resolves the issue of mixing random templating languages with xml. And perhaps the aliasing problem needs to be solved at a different layer, not in the grammars themselves.

You could imagine something like:

hljs.addComplexAlias("php", to: ["php","php-template"])

@joshgoebel
Copy link
Member Author

joshgoebel commented Feb 25, 2020

This touches on #2371 also.

Perhaps aliases should be more ambiguous. As as soon as you add asm as an extension/alias it could really refer to x86, ARM, AVR, etc... perhaps that's OK. and using asm should trigger auto-detect of all 3 languages.

So perhaps both PHP and PHP-template claim php as an alias and that "just works". I don't think that future feature should prevent merging this PR though - as I still feel this is a step in the right direction.

@egor-rogov
Copy link
Collaborator

I'd like to suggest that php and php-template are clearer than php and xml

No doubt.

PHP has historically been defined as "PHP/HTML"
Ruby and ERB are not interchangeable. Ruby code is Ruby code and ERB is a templating language based on Ruby.

ERB is lucky to have its own name. But for PHP you're like "well, I'm using PHP inside HTML, so I'm going to use... what's-his-name... php-html? no... ah, php-template! some day I'll memorize..."

So perhaps both PHP and PHP-template claim php as an alias and that "just works".
I don't think that future feature should prevent merging this PR though - as I still feel this is a step in the right direction.

Yeah, this is a way to go.
I tried to do something like this on the level of grammar, but it doesn't look right.

@joshgoebel
Copy link
Member Author

But for PHP you're like "well, I'm using PHP inside HTML, so I'm going to use... what's-his-name... php-html? no... ah, php-template! some day I'll memorize..."

Yeah, I do understand the problem you're trying to solve. :-) And I don't love vbscript-html either necessarily. Technically it's "Active Server Pages" I think [that allows for Javascript also]... I used that stuff 20 years ago or so...

@joshgoebel joshgoebel merged commit 0a5bbe4 into highlightjs:master Feb 25, 2020
joshgoebel added a commit that referenced this pull request Mar 3, 2020
- Adds a separate language to detect PHP templates to avoid overloading
  `xml`

XML never should have built-in PHP support in the first place as there
are any number of templating languages out there, and they all can't be
built into XML.

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

Labels

enhancement An enhancement or new feature language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discuss: How we handle the PHP templating language

2 participants