comment in a function call
Open, In Progress, LowPublic
Actions

Assigned To

Authored By

	matmarex
	Aug 13 2024, 1:37 PM

Description

Gerrit's syntax highlighting for PHP code breaks when encountering an apostrophe in a // comment in a function call. Everything from that apostrophe to the next one (wherever it appears in the file) is highlighted as a string literal.

Live example: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1049653/4/includes/Message/Message.php

I submitted that patch a few weeks ago to move that one comment because I thought it was an isolated issue, but since then I noticed the same problem in other files. Another example: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1062376/1/includes/specialpage/LoginSignupSpecialPage.php#797

Related Objects

Mentioned In: T395324: Gerrit trips on complicated string for PHP syntax highlighting

Event Timeline

matmarex created this task.Aug 13 2024, 1:37 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 13 2024, 1:37 PM

Daimona subscribed.Aug 13 2024, 1:45 PM

Thanks for tracking this down. This would be nice to fix, as the status quo makes it much more difficult to efficiently review patches in the Gerrit UI.

Using the second given example, I have reproduced the issue with highlight.js 11.9.0 and the following code:

$form->setAction(
      // Don't use $contlang->ucfirst() here.

Which renders:

highlight.js_11.9.0_php_single_quote.png (125×492 px, 8 KB)

The generated HTML is:

<span class="hljs-variable">$form</span>-&gt;<span class="hljs-title function_ invoke__">setAction</span>(
      // Don<span class="hljs-string">'t use $contlang-&gt;ucfirst() here.
</span>

Looks like the start of the comment is not detected, cause if I put a comment on its own line it is rendered wrapped within a <span class="hljs-comment />`.

It is still an issue in highlight.js main (85b20421a2620b765508f16289e0592e0563cfcb).

I filed https://github.com/highlightjs/highlight.js/issues/3995 but was closed without a fix.

Using https://github.com/highlightjs/highlight.js, the build is npm run build. The tests for PHP are under test/markup/php/ and the test suite can be run solely against the php test cases by setting ONLY_LANGUAGES env variable:

ONLY_LANGUAGES=php npm run test-markup

> highlight.js@11.11.1 test-markup
> mocha test/markup



  highlight() markup
    ✔ adding dynamic tests...

  php
    ✔ should markup attributes
    ✔ should markup case
    ✔ should markup comments
    ✔ should markup functions
    ✔ should markup namespace
    ✔ should markup numbers
    ✔ should markup php80
    ✔ should markup strings
    ✔ should markup titles


  10 passing (61ms)

The issue is the function can contains block comments (/* */) but can not contain line comments (// ) which is an easy fix.

PHP 8 attributes are similar but since they can't span multiple lines, you can't insert a line comment.

There is the same issue for PHP 7.4 arrows functions such as:

fn(
    $x, // C line comment
    /* C block comment */
    $y,
) => $x / $y;

Because internally the code to handle arrows function got copy pasted (I believe to add support for self).

Fix proposed at https://github.com/highlightjs/highlight.js/pull/4230

I have amended to also support comments starting with # which would fix upstream https://github.com/highlightjs/highlight.js/issues/3655 . It is quite easy:
https://github.com/highlightjs/highlight.js/compare/80f3fa7bee289cbdb21277da1f0e0e216559d6bd..6060d5c68794ed43f2e036d0607404b7e33264b8

I have further amended the pull request in order to support PHP8 attributes and to add a changelog entry: https://github.com/highlightjs/highlight.js/compare/6060d5c68794ed43f2e036d0607404b7e33264b8..7a6a1f2304275add8052903a29018f4bfeafba74

I have poked the pull request ( https://github.com/highlightjs/highlight.js/pull/4230#issuecomment-2802927830 )

https://github.com/highlightjs/highlight.js/pull/4230 has been merged by Upstream. That would be in highlight.js 11.11.2 whenever it is released after which we still need to bump it in Gerrit itself.

hashar mentioned this in T395324: Gerrit trips on complicated string for PHP syntax highlighting.May 27 2025, 1:16 PM

@Paladox any idea if this ever made its way to gerrit ?

In T372404#11551206, @TheDJ wrote:

@Paladox any idea if this ever made its way to gerrit ?

Nope as there's been no highlight.js release containing the fix. Last release was dec 2024 and the fix was merged in 2025.

	F58888685: highlight.js_11.9.0_php_single_quote.png
	Mar 21 2025, 12:53 PM

Gerrit's syntax highlighting for PHP code breaks when encountering an apostrophe in a // comment in a function callOpen, In Progress, LowPublicActions

Description

Related Objects

Event Timeline

Gerrit's syntax highlighting for PHP code breaks when encountering an apostrophe in a // comment in a function call
Open, In Progress, LowPublic
Actions