Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

skip highlighting large lockfiles on all instances, not just dotcom#63366

Merged
sqs merged 1 commit into
mainfrom
sqs/skip-hl-lockfiles
Jun 24, 2024
Merged

skip highlighting large lockfiles on all instances, not just dotcom#63366
sqs merged 1 commit into
mainfrom
sqs/skip-hl-lockfiles

Conversation

@sqs

@sqs sqs commented Jun 20, 2024

Copy link
Copy Markdown
Member

This is a slow and CPU-intensive task, and there is little value in syntax-highlighting lockfiles. Just disable this on all instances, not only dotcom.

Test plan

CI

Changelog

  • Syntax highlighting is disabled on lockfiles (such as package-lock.json) because it is CPU-intensive on these large files and very rarely desirable.

This is a slow and CPU-intensive task, and there is little value in syntax-highlighting lockfiles. Just disable this on all instances, not only dotcom.
@cla-bot cla-bot Bot added the cla-signed label Jun 20, 2024
@sqs sqs requested a review from a team June 24, 2024 04:24

@keegancsmith keegancsmith left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah agree, this shouldn't just be dotcom. Looking at the code though I don't see any checks on file size? If the file is small enough I don't understand why we don't highlight it. We have the content right there to check size.

Out of interest how did you come across this? This seems like something that would affect any large file, and maybe there is a strategy to improve our decision making that doesn't involve a dissallow list.

@sqs

sqs commented Jun 24, 2024

Copy link
Copy Markdown
Member Author

yeah agree, this shouldn't just be dotcom. Looking at the code though I don't see any checks on file size? If the file is small enough I don't understand why we don't highlight it. We have the content right there to check size.

Out of interest how did you come across this? This seems like something that would affect any large file, and maybe there is a strategy to improve our decision making that doesn't involve a dissallow list.

I came across it by looking for places where we are special-casing dotcom. I agree this should probably be based on file size. Filed: https://linear.app/sourcegraph/issue/SRCH-610/dont-syntax-highlight-large-files.

@sqs sqs merged commit 13cba57 into main Jun 24, 2024
@sqs sqs deleted the sqs/skip-hl-lockfiles branch June 24, 2024 15:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants