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

PHP syntax highlighting does not work without <?php #4865

Closed
1 of 4 tasks
harryqt opened this issue May 17, 2020 · 14 comments
Closed
1 of 4 tasks

PHP syntax highlighting does not work without <?php #4865

harryqt opened this issue May 17, 2020 · 14 comments
Assignees

Comments

@harryqt
Copy link

harryqt commented May 17, 2020

Preliminary Steps

Please confirm you have...

Problem Description

From last 2-3 days I noticed that PHP syntax highlighting stopped working not only on my repos but many repos I follow. After investigating I found that when code blocks start without <?php then php syntax highlighting does not generate.

Without PHP start tag:

chrome_2020-05-17_18-05-30

With PHP start tag:

chrome_2020-05-17_18-06-08

URL of the affected repository:

https://github.com/Dibbyo456/mysqli-database-class

Last modified on:

May 16, 2020

Expected language:

PHP

Detected language:

None

@lildude
Copy link
Member

lildude commented May 17, 2020

🤔 Are you sure this changed recently? I think this behaviour (which is part of markup, not Linguist) has been around for quite some time and there's even issue for it in the markup repo - github/markup#1230

It doesn't look like markup has been updated recently and I've not deployed an update to Linguist since March, so if you're 100% certain this worked before 16 May, please contact GitHub Support using the "Contact GitHub" link at the bottom of the page and report it to them as something outside of Linguist and markup is responsible for this change in behaviour.

@harryqt
Copy link
Author

harryqt commented May 17, 2020

🤔 Are you sure this changed recently?

Yes. A week ago everything was working just fine. I noticed it 5 days ago that markup wasn't showing up. At first I thought I broke it but then while browsing through other repos I realized the issue coming from Github itself.

please contact GitHub Support using the "Contact GitHub"

Okay. Will do. I will report back here. Thanks.

@mfn
Copy link

mfn commented May 18, 2020

This 200% worked without requiring <?php.

I daily write such fenced code blocks with PHP on the company repos and today I realized they're not colored anymore. This never needed an explicit PHP tag.

=> also contacted github support, this is quite a regression on whatever end

@johannesnagl
Copy link

same here. definitely changed

@Alhadis
Copy link
Collaborator

Alhadis commented May 19, 2020

Pretty sure the culprit is either recent changes to tree-sitter/tree-sitter-php, or a recent change to GitHub's closed-source fork of TextMate's highlighting engine. The change(s) in question more than likely concern the handling of injections, which PHP's grammar makes extensive use of.

That's about as much as I'm able to tell, as I've no knowledge of GitHub's internals or tree-sitter. However, it's been my experience that injections are almost always the catalyst of unexplained problems with language-grammars that make use of the feature.

/cc @maxbrunsfeld

@maxbrunsfeld maxbrunsfeld self-assigned this May 19, 2020
@maxbrunsfeld
Copy link

maxbrunsfeld commented May 19, 2020

Yeah, I think I broke this while optimizing GitHub.com's highlighting of embedded code blocks. It appears that there was some special casing of PHP to avoid this problem, and I failed to replicate that special casing in my parallelized implementation.

It isn't a Linguist issue, but let's leave this open until I ship the fix, which I think will happen within a week. Thanks for the reports, everyone, and sorry for the poor experience.

@Alhadis
Copy link
Collaborator

Alhadis commented May 19, 2020

It appears that there was some special casing of PHP to avoid this problem

Would that, perchance, have anything to do with injections being used instead of injectionSelector? Because I've noticed Atom isn't 100% consistent with the patterns defined on a grammar's injections object, and the patterns of a grammar which defines an injectionSelector string.

Another observation I've made is that injections don't affect the contents of a fixed-size capturing group:

­­injectionSelector: string.body

# This won't work:
name: string.quoted.double
match: /"([^"]+)"/
captures:
	1: name: string.body

# However, this will:
name:  string.quoted.double
begin: /"/
end:   /"/
contentName: string.body

@maxbrunsfeld
Copy link

No, the problem here wasn't tied to either the TextMate or Tree-sitter parsers, it was actually some PHP-specific preprocessing in GitHub itself.

@maxbrunsfeld
Copy link

$php.highlighting($within_code_blocks, IsNow(), FIXED_ON, "github.com")

// No need to include a `<?php` tag at the beginning.

?>

<p>
  Like before, you can <em>still</em> return to HTML mode.
</p>

@harryqt
Copy link
Author

harryqt commented May 29, 2020

@maxbrunsfeld The issue isn't fully resolved yet. When browsing a repo index, Github isn't highlighting php blocks but when viewing README.md explicitly code highlighting works.

Example:

Browsing repo (https://github.com/catfan/Medoo)

Browsing README.md (https://github.com/catfan/Medoo/blob/master/README.md)

@maxbrunsfeld
Copy link

@Dibbyo456 Thanks for the detailed report; I see the problem.

@maxbrunsfeld
Copy link

I believe the problem is just that GitHub has cached some incorrectly-rendered READMEs. I just forked the catfan/Medoo repo, and the README renders correctly in my fork.

I will look into busting this cache, but I don't have an ETA for when this will be fixed. Thanks for your patience.

@maxbrunsfeld
Copy link

I believe all of the incorrectly-rendered data is gone.

@harryqt
Copy link
Author

harryqt commented Jun 1, 2020

Yup. Thank you for taking care of this. 🤟

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants