Page MenuHomePhabricator
Differential D12147

Don't invoke pygmentize to highlight "txt" or "text" files
ClosedPublic
Actions

Authored by epriestley on Mar 24 2015, 8:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 3:52 PM
Unknown Object (File)
Wed, Nov 20, 3:30 PM
Unknown Object (File)
Wed, Nov 20, 12:37 PM
Unknown Object (File)
Fri, Nov 15, 6:55 PM
Unknown Object (File)
Mon, Nov 11, 11:05 PM
Unknown Object (File)
Mon, Nov 11, 8:34 PM
Unknown Object (File)
Fri, Nov 8, 7:10 AM
Unknown Object (File)
Sun, Nov 3, 11:44 PM
View All Files
Subscribers
epriestley
joshuaspence

Details

Reviewers
btrahan
Maniphest Tasks
T5644: Add controls to prevent `pygmentize` from saturating system resources
Commits
rPHU4c496670539c: Don't invoke pygmentize to highlight "txt" or "text" files
Summary

Ref T5644. Caught one more of these: we run pygmentize even when we know the file won't be highlighted.

Test Plan

Checked that we don't try to shell out to highlight text files.

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 29200.Mar 24 2015, 8:05 PM
epriestley retitled this revision from to Don't invoke pygmentize to highlight "txt" or "text" files.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley added a task: T5644: Add controls to prevent `pygmentize` from saturating system resources.
Herald added a subscriber: epriestley. · View Herald TranscriptMar 24 2015, 8:05 PM
btrahan accepted this revision.Mar 24 2015, 8:09 PM
btrahan edited edge metadata.
This revision is now accepted and ready to land.Mar 24 2015, 8:09 PM
joshuaspence added a subscriber: joshuaspence.Mar 24 2015, 8:09 PM

Maybe this should be a config duration option so that installs can customize it?

Closed by commit rPHU4c496670539c: Don't invoke pygmentize to highlight "txt" or "text" files (authored by epriestley, committed by epriestley). · Explain WhyMar 24 2015, 8:09 PM
This revision was automatically updated to reflect the committed changes.
epriestley added a comment.Mar 24 2015, 8:11 PM

Maybe this should be a config duration option so that installs can customize it?

Not sure what you mean, can you clarify?

joshuaspence added a comment.Mar 24 2015, 8:15 PM

Actually maybe this is okay. So I was thinking that an install might want to define additional languages that they don't wish to highlight. But I guess in that case they can just map those file extensions to text.

Revision Contents
Changeset List

PathSize
src/
markup/
syntax/
engine/
12 lines

Diff 29202

View Options

src/markup/syntax/engine/PhutilDefaultSyntaxHighlighterEngine.php

Loading...
Phabricator · Built by Phacility