[add-resource] New Plugin -- Server-side syntax highlighing with highlight.php

[iainsgillis]

Hi, Folks --

I would like to add my new plugin, which adds server-side syntax highlighting courtesy of the highlight.php library and the ShortCode Core plugin, to the Grav Repository.

The project lives here: iainsgillis/grav-plugin-highlight-php

FWIW, I wrote a series of blog posts about conceiving, designing, and releasing this plugin on my own site, starting with https://www.iainsgillis.com/essays/build-a-grav-plugin-1

[pamtbaau]

@iainsgillis, I've taken the liberty to create a review of your plugin.

Suggestions:

• HiglightPhpShortcode.php
  • Use shortcode with parameters

    [phphighlight=js]
    class Rectangle {
      constructor(height, width) {
        this.height = height;
        this.width = width;
      }
    }
    [/phphighlight]
    
    [phphighlight=js code="console.log('hello')"]
    

  For three reasons:
  • Only load shortcode for requested language(s), instead of 349. It saves a bit of performance.
  • No pollution of the shortcodes namespace with 349 extra names.
  • No need to search for 349 languages in Markdown while they are not used anyway.
• Use a fixed location for custom styles. I don't see a real added value for a customisable location.
  My suggestion would be to use folder: /user/data/hightlight-php/. I prefer the user/data/ folder to not pollute the user folder. Drop the folder property from the config.
• Use two config properties for styles: style for build-in styles and customStyle for a custom style. Add a None value to both.
  • User can now
    • Select a build-in style or None
    • Select a custom style or None
    • Select both build-in and custom style to override the build-in style.
  • Load each style (if value !== 'None') as separate asset with customStyle as last.
• Add a property in a page to enable/disable the plugin.
  I presume a page using snippets will be the exceptions instead of the rule. Test for it during onPageInitialized.

[iainsgillis]

Many thanks for this review -- I appreciate the suggestions, and the rationale. I'll make some revisions in the coming days.

On Mon, Mar 21, 2022, at 07:54, pamtbaau wrote: @iainsgillis <https://github.com/iainsgillis>, I've taken the liberty to create a review of your plugin. Suggestions: * HiglightPhpShortcode.php * Use shortcode with parameters `[phphighlight=js] class Rectangle { constructor(height, width) { this.height = height; this.width = width; } } [/phphighlight] [phphighlight=js code="console.log('hello')"]

`

…

For three reasons: * Only load shortcode for requested language(s), instead of 349. It saves a bit of performance. * No pollution of the shortcodes namespace with 349 extra names. * No need to search for 349 languages in Markdown while they are not used anyway. * Use a fixed location for custom styles. I don't see a real added value for a customisable location. My suggestion would be to use folder: */user/data/hightlight-php/*. I prefer the *user/data/* folder to not pollute the user folder. Drop the folder property from the config. * Use two config properties for styles: `style` for build-in styles and `customStyle` for a custom style. Add a `None` value to both. * User can now * Select a build-in style or None * Select a custom style or None * Select both build-in and custom style to override the build-in style. * Load each style (if value !== 'None') as separate asset with customStyle as last. * Add a property in a page to enable/disable the plugin. I presume a page using snippets will be the exceptions instead of the rule. Test for it during `onPageInitialized`. — Reply to this email directly, view it on GitHub <#3565 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACR666B2MJTTT3G5V5VFLC3VBB5TDANCNFSM5RGDWE7Q>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[pamtbaau]

@iainsgillis, Forgot to mention that the following condition will never be true, because the plugin will not be called when not enabled. It can be removed.

hightlight-php.php: line 58-60

if (!$this->config->get('plugins.highlight-php.enabled')) {
    return;
}

[iainsgillis]

@pamtbaau Many thanks for the review and the suggestions! Really appreciate your time.

[rhukster]

Sorry missed this one, added.