Skip to content

Fix bug with wrapLines that butchers highlighting #561

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

Merged
merged 2 commits into from
Oct 16, 2024

Conversation

jorgezreik
Copy link
Contributor

The Problem

When highlighting in JSX or TSX and using prism, template strings would sometimes fail to be highlighted correctly, as seen in the demo screenshot below.

2024-09-16 at 18 20 40

To reproduce, include any JSX node with multi-line text as well as a multi-line template string. You'll see that everything between the first and last lines of the template string is highlighted as plaintext instead of string.

This happens even though the demo for the underlying refractor package does not produce such an error.

2024-09-16 at 18 26 36

The Cause

The code for createLineElement in highlight.js modified the lineProps variable directly without copying it. This leads to lineProps at times taking on the className value of the currently processed line, which can then lead to other lines being wrapped with those classes.

What happened in the example above is that when the plaintext inside the JSX is processed, lineProps is butchered and then overrides the previously emitted string classes for the template string.

This code also leads to odd behavior when lineProps is specified but wrapLines is not, with only some lines being affected by lineProps and others not, seemingly at random.

The Fix

  1. We shallow copy the lineProps object to prevent it from being butchered.
  2. We only use the lineProps object when wrapLines is set to true, as otherwise (according to the README documentation) it has no effect.
  3. We merge the className field of lineProps with the generated classes to ensure both are present in the final output.

Testing

I have confirmed this fixes the issue above, while also preserving wrapLines functionality. Additionally, lineProps now has no effect on output when wrapLines is not specified.

2024-09-16 at 18 30 04

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.04%. Comparing base (b0d7714) to head (374e68a).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #561      +/-   ##
==========================================
+ Coverage   95.97%   96.04%   +0.07%     
==========================================
  Files          11       11              
  Lines         323      329       +6     
  Branches      123      130       +7     
==========================================
+ Hits          310      316       +6     
  Misses         12       12              
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jorgezreik
Copy link
Contributor Author

jorgezreik commented Sep 16, 2024

Will update tests soon! Tests are now updated :)

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

Successfully merging this pull request may close these issues.

2 participants