Page MenuHomePhabricator

Switching to pygmentize -V
Needs ReviewPublic

Authored by tycho.tatitscheff on Oct 17 2015, 1:07 PM.
Referenced Files
F13210750: D14297.diff
Fri, May 17, 5:10 AM
F13194685: D14297.diff
Sun, May 12, 9:46 PM
F13179183: D14297.diff
Wed, May 8, 9:00 PM
Unknown Object (File)
Mon, May 6, 8:19 AM
Unknown Object (File)
Sat, May 4, 6:26 PM
Unknown Object (File)
Fri, May 3, 3:20 AM
Unknown Object (File)
Wed, May 1, 7:21 PM
Unknown Object (File)
Sun, Apr 28, 5:26 PM



Fixes T9587.

It :

  • run pygmentize -V (instead of pygmentize -h ) which output a version sentence
    • if it fails, it will display the existing pygments.failed
    • it parses the sentence:
      • if the sentence is empty or don't match the regex /Pygments version (\d(\.\d)+)/, it will display a pygments.invalide
      • if the sentence match it will extract the version number:
        • if version isn't at least 2.0, it will display a pygments.outdated
        • else, it is fine 🌴 🍸

I also modularize the pht to extract pygmentize from the string and make message generalisable and thus translatable.

Test Plan

I basically tested quite all the steps (summary and message) to see I didn't break anything.


notfound_summary.png (58×1 px, 10 KB)

notworking_message.png (1×1 px, 106 KB)


notworking_summary.png (57×1 px, 9 KB)

notworking_message.png (1×1 px, 106 KB)

Note that the url was ../pygments.invalide in the screenshot. It was corrected in the last diff !


invalid_summary.png (60×1 px, 11 KB)

invalid_message.png (1×1 px, 98 KB)


outdated_summary.png (61×1 px, 12 KB)

outdated_message.png (1×1 px, 98 KB)

Diff Detail

rP Phabricator
Lint Passed
Tests Passed
Build Status
Buildable 8701
Build 10099: arc lint + arc unit

Event Timeline

tycho.tatitscheff retitled this revision from to [WIP] Switching to pygmentize -V.
tycho.tatitscheff updated this object.
tycho.tatitscheff edited the test plan for this revision. (Show Details)

@epriestley : something like this would be correct ?
Is there unit test I must write for this ?

cspeckmim added inline comments.

I don't think this will work. Check what the output of pygmentize -V is in order to be parsed; @epriestley gives examples from T9587#140655:

$ pygmentize -V
Pygments version 2.0.1, (c) 2006-2014 by Georg Brandl.

The version_compare will only be able to parse the 2.0.1 and not any of the rest of the line:

You will need to parse the version number out of $stdout prior to using it in version_compare in order for this to be a valid comparison.


This should probably list the existing version of pygmentize found, and the second sentence here might read better if phrased a little differently:

Upgrading pygmentize to version 2.0 or greater can improve performance in syntax highlighting.

I think these upgrade installations are too specific for a setup. Particularly in my case I do not have pip installed. The instructions on /config/edit/pygments.enabled/ indicate:

To install Pygments, visit and follow the download and install instructions.

Deferring to installation instructions which pygments itself instructs. The upgrade instructions should probably be similar.


Does this need to reference something or can a new issue identifier be created on-the-spot?


I'm not very familiar with reporting issues in Phabricator - I'm guessing this line adds Phabricator's environment path to the output,and probably useful to display for this purpose.

@cspeckmim thank you from the multiple advices. I will correct this soon(TM).

epriestley added a reviewer: epriestley.

See @cspeckmim's feedback.

This revision now requires changes to proceed.Nov 2 2015, 4:41 PM

I will push my work later this week.

tycho.tatitscheff edited edge metadata.
  • Taking comment into account : not actually yet tested (i intend to switch python version and see all works as axpected)
tycho.tatitscheff added inline comments.

That was also my thought.


Sems so : I grep for pygments.failed and only found one entry.


I'm not sure what would work best here. Off-hand I think this would fail if one of the version numbers was more than 1 digit:

version 2.0.19

As this regex is looking for single digits. Here's some psuedo-php/regex

// match the output looking for just "version x.x.x", capturing "x.x.x" in the match group
preg_match('/version ([\d|\.]+)/', $stdout, $match);
// strip out the "version ", should do error checking in case nothing matched!
$match[0] = $match[0].substring("version ".length);

That's not correct php but I'm not very familiar with string processing in php and don't have time at the moment to look up the proper way to do it.


Needs a space -> "available. Upgrading"


or (\d+\.\d+\.\d+)


Yea, that's what I was thinking but would need to test. I didn't know if the + would work inside a capture group (). Maybe PHP's version_compare has available a regex it uses internally that we can utilize?

This comment has been deleted.
tycho.tatitscheff edited edge metadata.
  • Fixing stuff
  • Fixing Lint and adding original version number
tycho.tatitscheff retitled this revision from [WIP] Switching to pygmentize -V to Switching to pygmentize -V.Nov 8 2015, 12:13 PM
tycho.tatitscheff updated this object.
tycho.tatitscheff edited the test plan for this revision. (Show Details)
tycho.tatitscheff added inline comments.

@epriestley : can I rename this to 'pygments.notfound' ??


Maybe I should not hardcode 2.0.0 and make a variable like $min_perf_version ?

This style issue apart what version should we wait to get @gd improvements in ?


I think it would be 2.1 !

Possibly we should wait until that makes it to a release and specifically recommend the release containing that fix ("Upgrade to 2.0.5 or newer...", or whatever the next release number is) since it's probably the highest-impact user-facing change between 1.4 and today for most users, albeit only for a subset of inputs.

Shall I put on hold till 2.1 appears ?


TODO : Shall be pygments

@epriestley : pygments is now 2.1.1 so the diff will actually make user upgrade to a version that fix T9587.

However it has been a long time since I worked on that. If it still interest you, I can rebase on master, retest and make changes according to some code review. Else, lets abandon this.