Page MenuHomePhabricator

Don't highlight very large files by default
ClosedPublic

Authored by epriestley on Mar 22 2015, 7:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 9:39 PM
Unknown Object (File)
Tue, Apr 16, 5:10 AM
Unknown Object (File)
Tue, Apr 16, 2:08 AM
Unknown Object (File)
Mon, Apr 8, 7:58 AM
Unknown Object (File)
Sat, Mar 30, 7:28 AM
Unknown Object (File)
Thu, Mar 21, 10:55 AM
Unknown Object (File)
Mar 18 2024, 6:26 PM
Unknown Object (File)
Feb 25 2024, 4:14 PM
Tokens
"Evil Spooky Haunted Tree" token, awarded by joshuaspence."Like" token, awarded by avivey.

Details

Summary

Ref T5644. See some discussion in D8040.

When a file is very large (more than 64KB of text), don't activate syntax highlighting by default. This should prevent us from wasting resources running pygmentize on enormous files.

Users who want the file highlighted can still select "Highlight As...".

The tricky part of this diff is separating the headers into "changeset" headers and "undershield" (rendering) headers. Specifically, a file might have these headers/shields:

  • "This file is newly added."
  • "This file is generated. Show Changes"
  • "Highlighting is disabled for this large file."

In this case, I want the user to see "added" and "generated" when they load the page, and only see "highlighting disabled" after they click "Show Changes". So there are several categories:

  • "Changeset" headers, which discuss the changeset as a whole (binary file, image file, moved, added, deleted, etc.)
  • "Property" headers, which describe metadata changes (not relevant here).
  • "Shields", which hide files from view by default.
  • "Undershield" headers, which provide rendering information that is only relevant if there is no shield on the file.
Test Plan
  • Viewed a diff with the library map, clicked "show changes", got a "highlighting disabled" header back with highlighting disabled.
  • Enabled highlighting explicitly (this currently restores the shield, which it probably shouldn't, but that feels out of scope for this change). The deshielded file is highlighted per the user's request.
  • Loaded context on normal files.

Diff Detail

Repository
rP Phabricator
Branch
uni44
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 4946
Build 4964: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

epriestley retitled this revision from to Don't highlight very large files by default.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
btrahan added inline comments.
src/applications/differential/parser/DifferentialChangesetParser.php
5

64kb is 64000 bytes i thought?

This revision is now accepted and ready to land.Mar 23 2015, 5:30 PM

Oh, sorry, the actual limit is 256KiB.

(An earlier version of this change used a 64KB limit, but looking through Phabricator we had a few files that were that large legitimately.)

This revision was automatically updated to reflect the committed changes.