search

scrivo / highlight.php

A port of highlight.js by Ivan Sagalaev to PHP
BSD 3-Clause "New" or "Revised" License
690 stars 44 forks source link

Where is highlight.php v10.x/v11.x? #73

Open allejo opened 4 years ago

allejo commented 4 years ago

For anyone curious and before anyone asks, highlight.php 10.x/11.x is planned and development will start in the coming weeks. I first need to finish off my semester and a few other commitments.

highlight.php 10 will require PHP 5.6+ (see #55)

The current estimated release of highlight.php v10/11 is Winter 2021

Why the delay?

highlight.js v10 introduced some breaking changes where grammars can no longer be represented by just JSON; grammars now support JS callbacks. While not many grammars are using this feature as of yet, I'm hesitant to dedicate myself to manually porting JS to PHP.

Does this mean highlight.php is dead?

No. Not at all. I do have some ideas for automatically translating JS to PHP to address this problem. I simply have not had time to work on this.

It's been over a year since this issue was created. Why haven't you done it yet?

I've had other projects, work, and school that have required my attention/focus/time. Feel free to sponsor me to work on this project :smile: :heart:

joshgoebel commented 4 years ago

Very curious to see how you handle the dynamism that's possible now.

allejo commented 4 years ago

Very curious to see how you handle the dynamism that's possible now.

Me too :sweat_smile:

joshgoebel commented 4 years ago

I used it in the new SHEBANG mode too since now it's actually possible (for the first time) to check if a match starts at the beginning of the content, not just the beginning of the line.

phpfui commented 4 years ago

For version 10, can we change the HighlightUtilities namespace to Hightlight\Utilities and make Highlight\Utilities a static class with the functions as static methods? I think this is a better organization. No reason for two namespaces.

Also I think there should be a src directory to make the package a bit more standard.

Neither of these changes should really impact many users and it should be an easy change.

Happy to submit a PR with the above changes, I just don't like submitting PRs with changes such as this without buy in from the author. But V10 would be the time to make the change.

allejo commented 4 years ago

For version 10, can we change the HighlightUtilities namespace to Hightlight\Utilities and make Highlight\Utilities a static class with the functions as static methods? I think this is a better organization. No reason for two namespaces.

The only reason I opted to have a separate namespace was so we can keep the Highlight namespace as loyal as possible to a port of the highlight.js project. Let me sit on this one for a bit.

Is there an advantage to having these functions under a static class? As functions, you can currently import them like use function \HighlightUtilities\getStyleSheetFolder() and even give them aliases. This is a 5.6+ feature, so considering the min requirement of 5.6 for 10.x, this would continue to work.

Also I think there should be a src directory to make the package a bit more standard.

Neither of these changes should really impact many users and it should be an easy change.

Happy to submit a PR with the above changes, I just don't like submitting PRs with changes such as this without buy in from the author. But V10 would be the time to make the change.

Yea, I've been wanting to change this project into a PSR-4 setup for a while, just never got around to it. Sure you can make that PR, continue to keep two separate namespaces for now though for Highlight and HighlightUtilities.

phpfui commented 4 years ago

Actually after I wrote this, I realized you only need one namespace. You put in a class called Utilities with static methods. So instead of \HighlightUtilities\getStyleSheetFolder(), it becomes \Highlight\Utilities::getStyleSheetFolder().

It is a very easy port, just search and replace really and uses one namespace.

As for PSR-4, not sure it makes much of a difference vs PSR-0 if you put things in a src directory. The problem with PSR-0 and no source directory is that you can't programatically tell what belongs to the library and what is just support code. This may not matter to most users, but if you are using a super fast autoloader, then you need a mapping between the namespace and the file system, and you have no easy way to populate the file system without including a bunch of junk. Anyway, a src is best practice and does not really impact anything.

Let me know your thoughts on the Utilities class and static methods. I think it is the best approach.

Thanks thanks for responding. I use this package for my PHPFUI website to display the source and it works great, although sometimes a bit slow on larger files.

phpfui commented 4 years ago

So I started moving to PSR-4 and replacing the HighlightUtilities namespace. Turns out you just change \HightlightUtilities\ to \Highlight\Utilities:: and you are done!

But I noticed that the project is PHP 5.4 (barf!). How about PHP 7.1 or 7.2? 7.1 is no longer supported, but a decent version. 7.2 adds nullable types, which may not be needed. Leave V9 for older versions, and V10 would be 7.1 or higher.

Also would want to upgrade PHPUnit, which is fairly easy.

allejo commented 4 years ago

Before we get a bit too carried away...

I'm not really sold on the benefits of moving the HightlightUtilities namespace to an abstract class, \Highlight\Utilities. I really do not want to deviate too much from origin project (highlight.js) and clutter up the main Highlight namespace, which is why it was added as a separate one. What if highlight.js introduces a Utilities down the line? I want to make the separation of the components of this project clear, what is loyal to highlight.js, and what isn't.

As for the PHP version, v10 is currently planning on targeting 5.6; see the first post in this issue and issue #55. Since I'm actively working on a WordPress plugin that incorporates this library and there are no critical features that we need from PHP 7.x, I have no problems supporting 5.6 for a while longer. highlight.js is planning to have a quicker release schedule, so there'll be more opportunities to bump the minimum PHP version without having to wait for a major bump.

phpfui commented 4 years ago

It is your library. I am simply suggesting some common techniques that work well. Classes with static methods work better than functions in namespaces as it allows for a simpler autoloader. When you start getting into larger projects, autoloader speed becomes an issue. A PSR-0 autoloader is super fast. Well structured PSR-4 libraries can be converted to PSR-0 easily. My autoloader takes 0 bytes of memory and has extremely low computational overhead (no array lookups). But it does not work well with namespaced functions (looking at you GuzzleHttp).

I am not big fan of taking more namespaces than needed, and you really only need one. I did delete the Autoloader class, so as far as classes in the Highlighter namespace that are not in the JS version, it is a wash. It is easy enough to rename Utilities if it comes to that.

The alternative might be to do a static method class in HighlightUtilities, but then you start duplicating names (ie. \HightlightUtilities\Utilities::methodName).

The biggest improvement is the src file structure, That will work with PSR-0 or 4.

Also the current standard for PHP libraries (Laravel and PHPUnit for example) is to just support current versions of PHP (meaning 7.2 currently). This encourages people to continue to upgrade, which benefits the entire community. Typed method parameters, typed return values, type properties and nullable types all help to insure your code works as expected. Typeless languages are simply not as good as more strongly typed languages. There are reasons why languages have historically added more and more type checking as the language progresses. FORTRAN, C, PHP and even the lowly JavaScript have all added more type checking as they evolved. But 5.6 is better than 5.3 for sure.

Let me know what you want to do and happy to update the PR. My biggest things are a src directory and no namespaced functions. PHP 7.2 is just me trying to push the PHP community into the 1990s of computer language design. I personally plan to bump the minimum PHP requirements in all my PHP projects to only currently supported versions when PHP 8 ships. Keeping up with PHP releases provides huge speed improvements, and I have seen noticeable speed improvements when ever I upgrade PHP. YMMV.

And thanks for providing this library!

localheinz commented 3 years ago

@allejo

Is there anything we can do to support the upgrade?

Any help needed on the code, or perhaps some sponsoring?

phpfui commented 3 years ago

I am not sure what else is in 10, but happy to test tonight. I will see if I can update to dev-master for my http://www.phpfui.com site. Will be publicly viewable for many php files.

On Thu, Jun 3, 2021, 5:51 PM Andreas Möller @.***> wrote:

@allejo https://github.com/allejo

Is there anything we can do to support the upgrade?

Any help needed on the code, or perhaps some sponsoring?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/scrivo/highlight.php/issues/73#issuecomment-854204583, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYW6S5HVLZLP5473EJOAITTQ72N3ANCNFSM4MYJOBTA .

phpfui commented 3 years ago

I upgraded phpfui.com to dev-master and WOW! It is much faster. The largest file timed out before (170K) and everything seems super snappy now. I would recommend an immediate release of V10. It was plug in compatible. I did not have to change anything.

Here are some example pages to check out. The second one did not load before:

You are looking at the actual code that is running on the site. The site renders the documentation real time, it is not stored, but generated on request.

Also, if you want to include a full class documentation link in the readme file, use this link: http://phpfui.com/?n=Highlight It includes all the namespaces, classes and readme. Since PHPFUI uses this package, it will always be documented on PHPFUI, as it always documents the classes it uses at a minimum (plus some other libraries like PhpParser, which is totally awesome).

joshgoebel commented 3 years ago

I upgraded phpfui.com to dev-master and WOW! It is much faster.

Are you simply referring the the latest highlight.php available from GitHub?

@allejo Did you make a release with that huge speed fix a while back? If not prolly should.

phpfui commented 3 years ago

Yes. I specified dev-master in composer.json. Was previously using v9.18.1.6. You can check out the commit here: https://github.com/phpfui/PHPFUIWebsite/commit/b19a8680834b596f1d5f7267c12a426ad5b43d99#diff-f37acfaa6b11f575a9a6f41a75fa73a61d0f8ebc2c9b8cddc215d8aca10e44f5

I believe there was a huge speed fix in a regex, but think it is only in 10. My experiment would confirm that.

I think 10 should be released ASAP.

joshgoebel commented 3 years ago

I believe there was a huge speed fix in a regex, but think it is only in 10.

Indeed, but that's not what you're talking about if you're using just the master branch. There is no v10 code here yet. There was a BIG speed fix a while back, perhaps a release was just never cut. In which case it should be, but it would be another v9 release, not 10 or 11.

phpfui commented 3 years ago

composer.json seems to be using psr-4 autoloading, and there is a src directory. So I think that is 10, as I put in a PR for that. Also see this:

    "extra": {
        "branch-alias": {
            "dev-master": "10.0-dev"
        }
    }

It could be 9, but what ever, it should be released. I don't really care, we need to get updated code out there and not be held hostage to quaint notions of needing backward compatibility back to the late Ming dynasty.

It works, ship it.

joshgoebel commented 3 years ago

It could be 9, but what ever, it should be released.

It is 9. And I agree with releasing - if the critical performance fix hasn't shipped, it should.

not be held hostage to quaint notions of needing backward compatibility back to the late Ming dynasty.

I have no idea what this is referring to. Older PHP versions or desire for backwards compatibility is NOT the reason there is no v10 or v11 release yet.

allejo commented 3 years ago

Is there anything we can do to support the upgrade?

Any help needed on the code, or perhaps some sponsoring?

I'd definitely welcome a helping hand with starting to port core highlight.js changes to the PHP codebase :+1: If you have time to spare for contributions, let me know and I'll be glad to discuss the porting process.

Anyone sponsoring my time to work on this to catch up would be nice :smile: Regardless of sponsorships, I do intend on working on this in the coming weeks (hopefully) :crossed_fingers:

I upgraded phpfui.com to dev-master and WOW! It is much faster. The largest file timed out before (170K) and everything seems super snappy now. I would recommend an immediate release of V10. It was plug in compatible. I did not have to change anything.

I wish I could tell you I worked hard at making things fast :sweat_smile: But as mentioned, the master branch (i.e. v10-dev) is pretty much the same as the 9.18 series with the exception of some reorganization of files.

As Josh mentioned, there was a significant speed fix but that exists in the 9.18 series. Maybe phpfui.com wasn't using the latest 9.18 series?

@allejo Did you make a release with that huge speed fix a while back? If not prolly should.

The speed fix you are referring to was released in v9.18.1.5. There is another unrelated speed fix in v9.18.1.6, but that is only specific to the splitCodeIntoArray() utility function.

joshgoebel commented 3 years ago

The speed fix you are referring to was released in v9.18.1.5. There is another unrelated speed fix in v9.18.1.6,

Well then I have no ideas here. :)

phpfui commented 3 years ago

I did some more research on why I am seeing a performance improvement from v9.18.1.6 to master. The only thing that I see is this line:

preg_match_all($this->source, $str, $results, PREG_SET_ORDER | PREG_OFFSET_CAPTURE, $this->lastIndex);

PREG_SET_ORDER has been removed in master. You can see the change here: http://phpfui.com/?n=Highlight&c=RegEx&p=g#

I get a memory error on v9.18.1.6, but master runs quickly.

The other change I see is that master uses PSR-4 autoloading with a src directory, while v9.18.1.6 is using PRS-0. Not sure where this was introduced (I put in the PR a long time ago), but I was under the impression that PSR-4 would be in 10, which is why I was advocating for its release.

In any case, I think v9.18.1.7 would be in order based on master branch. Seems good to me.

joshgoebel commented 3 years ago

Could you make a quick edit to confirm which change is the cause of the performance difference?

phpfui commented 3 years ago

Confirmed the last changes to Highlight\RegEx::exec method fixed a memory issue in previous version. Actually it is doing a bit more than just the one option, also changed from preg_match_all to just preg_match. I had to revert the entire method, as it was not a change to just remove the one option.

allejo commented 3 years ago

I'm honestly not sure where you're finding the differences. In 9.18.1.4, there was the inefficient function call that PREG_SET_ORDER and preg_match_all. This was changed fixed in 9.18.1.6 to use preg_match and removed PREG_SET_ORDER. This change was then ported forward to the master branch.

master aka v10-dev

https://github.com/scrivo/highlight.php/blob/c66fe5cb6a70d14da39948ae57320eb08186891b/src/Highlight/RegEx.php#L79

v9.18.1.6 tag

https://github.com/scrivo/highlight.php/blob/44a3d4136edb5ad8551590bf90f437db80b2d466/Highlight/RegEx.php#L79

v9.18.1.4 tag

https://github.com/scrivo/highlight.php/blob/ee8007a215a27cb9c078e0328fb5de901d74ef9b/Highlight/RegEx.php#L79

phpfui commented 3 years ago

The only thing I can think of is a composer failure. I have seen that before where the wrong version gets installed due to cache issues. Master changed to PRS-4 and maybe that refreshed the cache?

phpfui commented 1 year ago

Where are we with V10? I have been running dev-master in production for almost a year and it works great. No issues.

I think you can release master as V10 at this point.

westonruter commented 1 year ago

The current estimated release of highlight.php v10/11 is Winter 2021

I think this may be a bit out of date.