Add missing PHP code mixing within <style> tags by osumko · Pull Request #350 · atom/language-php

osumko · 2019-01-21T15:32:41Z

Description of the Change

CSS one of the required languages that can be mixed within PHP files along with HTML and Javascript, needed to embed specific code based on conditions.

HTML mixing <div><?php echo $title ?></div> and JS mixing <script type="text/javascript"><?php echo $var ?></script> inline or within blocks have been already implemented long time ago. But CSS support for <style> tags still missing.

CSS mixing is required only for php type files (php5, phtml, etc.) and only within <style> tags.
Mixing inside .css files, as well as .html and .js, is not allowed.

Alternate Designs

The only way to mixing css styles without breaking syntax highlighting is to duplicate whole <style> tags with different content or force to use external stylesheets, which is not always applicable when using templates, MVC pattern, when avoiding render blocking, etc.

Benefits

PHP parser not depends on IDE or any editor syntax highlighting. But this pull request adds support for correct syntax recognizing inside Atom, VS Code and many other editors that uses Atom's grammar, which needed for syntax linters, intellisense, code snippets and suggestions to work as well inside mixing content.

Possible Drawbacks

VS Code, for which this pull request was made initially, uses modified grammar rules.
VS Code rule L:(source.css - (meta.embedded.block.php | meta.embedded.line.php)) can be loaded from /php/ folder extension, but Atom rules may require modifications to work only within php files. This may be source.css.embedded.html if there is exists one.

Applicable Issues

None.

Example

<style type="text/css">
<?php if ( empty( $comments ) ) { ?>
.comments {
    display: block;
}
<?php } ?>
</style>

Allow to recognize syntax when embedding php in css style tags within php files, similar to html and js script tags, e.g.: ``` <style type="text/css"> <?php if ( empty( $comments ) ) { ?> .comments { display: block; } <?php } ?> </style> ```

rsese · 2019-01-25T03:51:47Z

Thanks @esemlabel! Can you edit the issue body with the pull request template details? The format and information asked for is helpful for anyone that has to review a pull request.

KapitanOczywisty · 2019-09-08T19:56:16Z

@rsese Pull request was updated a long time ago and should be merged in next batch. I've checked this change on atom and vscode - works as expected.

This fixes: microsoft/vscode#33519

rsese · 2019-09-09T16:38:09Z

Thanks for the heads up @KapitanOczywisty, there aren't notification for edits so missed the pull request body update (sorry about that @esemlabel!).

Now that the PR body has been updated to use the template, I noticed there aren't any tests, is that something you can add @esemlabel? If so, can you drop a comment after that's done so we get a notification?

should be merged in next batch

Just wanted to mention that as with other pull requests, we can't promise any specific timeframe for reviewing or merging a particular PR.

osumko · 2019-09-10T09:09:25Z

@rsese , what kind of test do you mean?
Currently, I'm using this patch for the last year in my development and didn't face up with any issues.

KapitanOczywisty · 2019-09-10T12:29:27Z

@esemlabel Something like this, just for style tag

language-php/spec/html-spec.coffee

Lines 83 to 111 in e3281a8

    
               it 'does not tokenize PHP tag syntax within PHP syntax itself inside HTML script tags (regression)', -> 
        
                 lines = grammar.tokenizeLines ''' 
        
                   <script> 
        
                   <?php 
        
                     /* 
        
                       ?> 
        
                       <?php 
        
                     */ 
        
                   ?> 
        
                   </script> 
        
                 ''' 
        
                 expect(lines[0][0]).toEqual value: '<', scopes: ['text.html.php', 'meta.tag.script.html', 'punctuation.definition.tag.html'] 
        
                 expect(lines[0][1]).toEqual value: 'script', scopes: ['text.html.php', 'meta.tag.script.html', 'entity.name.tag.script.html'] 
        
                 expect(lines[0][2]).toEqual value: '>', scopes: ['text.html.php', 'meta.tag.script.html', 'punctuation.definition.tag.html'] 
        
                 expect(lines[1][0]).toEqual value: '<?php', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'punctuation.section.embedded.begin.php'] 
        
                 expect(lines[2][0]).toEqual value: '  ', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'source.php'] 
        
                 expect(lines[2][1]).toEqual value: '/*', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'source.php', 'comment.block.php', 'punctuation.definition.comment.php'] 
        
                 expect(lines[3][0]).toEqual value: '    ?>', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'source.php', 'comment.block.php'] 
        
                 expect(lines[4][0]).toEqual value: '    <?php', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'source.php', 'comment.block.php'] 
        
                 expect(lines[5][0]).toEqual value: '  ', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'source.php', 'comment.block.php'] 
        
                 expect(lines[5][1]).toEqual value: '*/', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'source.php', 'comment.block.php', 'punctuation.definition.comment.php'] 
        
                 expect(lines[6][0]).toEqual value: '?', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'punctuation.section.embedded.end.php', 'source.php'] 
        
                 expect(lines[6][1]).toEqual value: '>', scopes: ['text.html.php', 'meta.tag.script.html', 'source.js.embedded.html', 'meta.embedded.block.php', 'punctuation.section.embedded.end.php'] 
        
                 expect(lines[7][0]).toEqual value: '</', scopes: ['text.html.php', 'meta.tag.script.html', 'punctuation.definition.tag.html'] 
        
                 expect(lines[7][1]).toEqual value: 'script', scopes: ['text.html.php', 'meta.tag.script.html', 'entity.name.tag.script.html'] 
        
                 expect(lines[7][2]).toEqual value: '>', scopes: ['text.html.php', 'meta.tag.script.html', 'punctuation.definition.tag.html']

You can run test in atom using ctrl+shift+y when in language-php folder

osumko · 2019-09-10T15:06:58Z

@KapitanOczywisty,
Unfortunately, I'm not familiar to coffee script or any other exposing tokenized lines technique.

KapitanOczywisty · 2019-09-10T16:02:37Z

Merge me or copy updated file
https://github.com/KapitanOczywisty/language-php-style/commit/bf95a9cb470ac7531f659f144c2d40e5035d7b53

https://github.com/KapitanOczywisty/language-php-style/blob/bf95a9cb470ac7531f659f144c2d40e5035d7b53/spec/html-spec.coffee#L112-L138

osumko · 2019-09-10T16:55:19Z

Done.

KapitanOczywisty · 2019-09-15T19:40:39Z

VScode team agreed to merge this change in update script downstream. I've checked atom and there is no source.css scope, so change would have no effect in atom itself. VScode is overriding injections, so this change would need to be done there anyway.

This pr can be closed, and sorry that I didn't check scopes before I dug out this.

rsese · 2019-09-18T20:46:32Z

Oh ok no problem, thanks for clarifying - since you're the pull request author @esemlabel does this sound good to you?

osumko · 2019-09-19T09:00:40Z

I'm good.

rsese · 2019-09-19T16:34:07Z

Thanks, I'll go ahead and close this out 👍

Php inside css style tags

a267a1f

Allow to recognize syntax when embedding php in css style tags within php files, similar to html and js script tags, e.g.: ``` <style type="text/css"> <?php if ( empty( $comments ) ) { ?> .comments { display: block; } <?php } ?> </style> ```

rsese added the more-information-needed label Jan 25, 2019

osumko changed the title ~~Php inside css style tags~~ Add missing php code mixing within <style> tags Jan 25, 2019

osumko changed the title ~~Add missing php code mixing within <style> tags~~ Add PHP code mixing within <style> tags Jan 25, 2019

osumko changed the title ~~Add PHP code mixing within <style> tags~~ Add missing PHP code mixing within <style> tags Jan 25, 2019

KapitanOczywisty approved these changes Sep 8, 2019

View reviewed changes

KapitanOczywisty mentioned this pull request Sep 8, 2019

PHP is not linted inside the style tag? bmewburn/vscode-intelephense#667

Closed

rsese added needs-testing and removed more-information-needed labels Sep 9, 2019

Add files via upload

faf4535

KapitanOczywisty mentioned this pull request Sep 10, 2019

Fix PHP in HTML style tags microsoft/vscode#80657

Merged

rsese closed this Sep 19, 2019

rsese removed the needs-testing label Sep 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing PHP code mixing within <style> tags#350

Add missing PHP code mixing within <style> tags#350
osumko wants to merge 2 commits intoatom:masterfrom
osumko:patch-1

osumko commented Jan 21, 2019 •

edited

Loading

Uh oh!

rsese commented Jan 25, 2019

Uh oh!

KapitanOczywisty commented Sep 8, 2019

Uh oh!

rsese commented Sep 9, 2019

Uh oh!

osumko commented Sep 10, 2019

Uh oh!

KapitanOczywisty commented Sep 10, 2019 •

edited

Loading

Uh oh!

osumko commented Sep 10, 2019

Uh oh!

KapitanOczywisty commented Sep 10, 2019

Uh oh!

osumko commented Sep 10, 2019

Uh oh!

KapitanOczywisty commented Sep 15, 2019 •

edited

Loading

Uh oh!

rsese commented Sep 18, 2019

Uh oh!

osumko commented Sep 19, 2019

Uh oh!

rsese commented Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

osumko commented Jan 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Applicable Issues

Example

Uh oh!

rsese commented Jan 25, 2019

Uh oh!

KapitanOczywisty commented Sep 8, 2019

Uh oh!

rsese commented Sep 9, 2019

Uh oh!

osumko commented Sep 10, 2019

Uh oh!

KapitanOczywisty commented Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osumko commented Sep 10, 2019

Uh oh!

KapitanOczywisty commented Sep 10, 2019

Uh oh!

osumko commented Sep 10, 2019

Uh oh!

KapitanOczywisty commented Sep 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsese commented Sep 18, 2019

Uh oh!

osumko commented Sep 19, 2019

Uh oh!

rsese commented Sep 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

osumko commented Jan 21, 2019 •

edited

Loading

KapitanOczywisty commented Sep 10, 2019 •

edited

Loading

KapitanOczywisty commented Sep 15, 2019 •

edited

Loading