Skip to content

Fix hljs highlighting for safari browser #11772

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix hljs highlighting for safari browser #11772

wants to merge 1 commit into from

Conversation

BarkingBad
Copy link
Contributor

No description provided.

@BarkingBad BarkingBad self-assigned this Mar 16, 2021
@BarkingBad BarkingBad linked an issue Mar 16, 2021 that may be closed by this pull request
Missing syntax highlighting for code snippets in Safari browser #11771
Closed
@BarkingBad BarkingBad marked this pull request as ready for review March 16, 2021 14:42
@BarkingBad
Copy link
Contributor Author

Before:
obraz
After:
obraz

A tried to change regexes to be look behind free as safari browser doesn't support that yet. I used a trick proposed here. Could you @TheElectronWill look into them whether they are correct?

TheElectronWill
TheElectronWill reviewed Mar 17, 2021
View reviewed changes
scaladoc/resources/dotty_res/scripts/hljs-scala3.js
@@ -36,7 +36,7 @@ function highlightDotty(hljs) {
function titleFor(name) {
return {
className: 'title',
begin: `(?<=${name} )${id.source}`
begin: `((?:${name} )(${id.source})`
Copy link
Contributor

Choose a reason for hiding this comment

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

missing parenthesis?

@TheElectronWill
Copy link
Contributor

TheElectronWill commented Mar 17, 2021 •
edited
Loading

I'm afraid this simple fix won't work. I have a test file in this repo that you can use.

  • changing id this way highlights the method names incorrectly: in def func_= the = will not be included in the function's name

  • changing titleFor breaks the highlighting of "titles", i.e. the names of variables and methods aren't highlighted anymore.
    example:
    image
    becomes
    image

  • changing ENUM_CASE changes how cases are detected, they end up as type instead of title. And in case A, B, A and B won't be labelled the same way.

  • changing the markdown links this way includes the label into the url, which is a bit weird semantically. If you underline the links you see the difference :
    image becomes image

TheElectronWill
TheElectronWill suggested changes Mar 17, 2021
View reviewed changes
Copy link
Contributor

@TheElectronWill TheElectronWill left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There should be a better way of avoiding the look-behinds. I'll try some workarounds 🙂
Related discussion in hljs repo

@BarkingBad
Copy link
Contributor Author

I'll try some workarounds

Would you like to overtake that issue?

@TheElectronWill
Copy link
Contributor

TheElectronWill commented Mar 18, 2021 •
edited
Loading

Would you like to overtake that issue?

I'll be glad to do it! I'll assign myself if that's ok with you

@BarkingBad
Copy link
Contributor Author

BarkingBad commented Mar 18, 2021 •
edited
Loading

No problem, thank you for your support :D

I think we can close that PR since it's on my fork and it would be hard to cooperate, and when you are ready, you'll open a fresh new one.

@BarkingBad BarkingBad closed this Mar 18, 2021
@TheElectronWill TheElectronWill mentioned this pull request Mar 21, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheElectronWill TheElectronWill TheElectronWill requested changes

Assignees

@TheElectronWill TheElectronWill

@BarkingBad BarkingBad

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing syntax highlighting for code snippets in Safari browser
2 participants
@BarkingBad @TheElectronWill