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

Syntax highlighting without inline styles #2

Open
Nessworthy opened this issue Nov 9, 2016 · 4 comments
Open

Syntax highlighting without inline styles #2

Nessworthy opened this issue Nov 9, 2016 · 4 comments

Comments

@Nessworthy
Copy link
Collaborator

Discussion points:

  • Client Side vs Server Side
  • Choice of Library

Other points to consider:

  • User comments and documentation use different highlighters, ideally these will want to be highlighted using the same source.
@kelunik kelunik self-assigned this Nov 9, 2016
@kelunik
Copy link
Collaborator

kelunik commented Nov 9, 2016

It's pretty easy, I'll push something soon.

@morrisonlevi
Copy link
Owner

It's a private repository but collaborators should have access: https://github.com/kelunik/php-highlight

@kelunik
Copy link
Collaborator

kelunik commented Nov 11, 2016

We already use class names in the user notes instead of inline styles. Well, kind of pretty hacky. https://github.com/php/web-php/blob/36168258a227b83926341c37eec0c151f8335b6e/include/layout.inc#L14-L49

@Sobak
Copy link

Sobak commented Dec 14, 2016

Hi!

I'd like to pipe, if I could. As I also thought about that, let me summarise what I found:

  1. I'm going to add something like ClientSideHighlighter in next stable version of PhD. A highlighter class which would return valid wrappers possible to be handled by projects like prism.js

  2. It will be shameless advertisement of a project written by a friend of mine, but I'd like to mention it, just to get you know what our options are. There is a modern, but also very fast (more than 2x faster than GeSHi on every tested case... if only GeSHi means everything and still can be used as a reference today). https://github.com/kadet1090/keylighter (it has minimal requirement of PHP 5.5 and for why is that important, see below)

  3. And now, the sad part comes. PHP version requirements:

  • PHP.net mirrors run mostly PHP 5.3 but they are not responsible for running PhD and thus for rendering our code snippets
  • PhD is run on about 5 servers from our stock, but there are two we should be interested in (rest of them doesn't render the Manual in web format AFAIK or can still use default highlighter [e.g. PEAR docs])
    • rsync box - this is from mirrors get their built versions of the Manual. I don't know what PHP version it runs, but I doubt it's very old
    • euk2 box (docs.php.net) - in case we want to switch our syntax highlightning, this box needs to have it switched as well, so that we can spot bugs. It runs PHP 5.6.29 as of yesterday. @kadet1090 highlighter's requirement of PHP 5.5 is already met on it. @kelunik's project requires PHP 7, but I believe these requirements can be lowered in case we choose it.
  • Highlightning user comments:
    • Each mirrors highlightings them independently so:
      • we choose new-shiny-awesome server-side highlighter for the manual and leave highlight_string() for the manual notes - it would be acceptable IMO, as notes doesn't have any kind of tags enclosing code snippets except for PHP tags. Which are very often unclosed and have text inside. And fixing 25k notes is a no-go. Actually, I don't know about other client-side highlighters but prism.js wouldn't be able to highlight them as well.
      • we choose client side highlighter with its all advantages and disadvantages, but we don't need to care about that anymore

So those are my 2cts (and, yeah, I kinda like nested lists...)

morrisonlevi pushed a commit that referenced this issue Jun 29, 2022
morrisonlevi pushed a commit that referenced this issue Jun 29, 2022
morrisonlevi pushed a commit that referenced this issue Jun 29, 2022
morrisonlevi pushed a commit that referenced this issue Jun 29, 2022
morrisonlevi pushed a commit that referenced this issue Aug 30, 2022
* .gitignore: Remove redundant entry

* Minor optimizations with ternary operators

* Use `const` instead of `define()` where appropriate

`const` is quite faster because of the compile-time optimizations. Because the replaced statements are not declaring constant conditionally, it's safe to use `const` in all of these places.

Closes phpGH-608.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants