Skip to content
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

Remove regex features that Safari does not support #1976 #1980

Merged
merged 2 commits into from
Jul 8, 2021

Conversation

iamleeg
Copy link
Contributor

@iamleeg iamleeg commented Jul 5, 2021

Safari doesn't support lookahead/lookbehind in regex, so the syntax highlighting had broken Safari support. I've changed the regex so it doesn't use them, which obviously means it doesn't match the same things any more so I've basically broken syntax highlighting. @sergioloporto hoping your regex skills can help design a highlight implementation that will work, any ideas?

Here's the compatibility matrix: https://caniuse.com/js-regexp-lookbehind

@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2021

Codecov Report

Merging #1980 (1defc3d) into main (b3b1105) will decrease coverage by 19.35%.
The diff coverage is 87.50%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1980       +/-   ##
===========================================
- Coverage   75.53%   56.18%   -19.36%     
===========================================
  Files          24       76       +52     
  Lines         928     2645     +1717     
  Branches      147      842      +695     
===========================================
+ Hits          701     1486      +785     
- Misses        227     1159      +932     
Impacted Files Coverage Δ
...ion/curator-service/ui/src/components/ViewCase.tsx 81.33% <87.50%> (ø)
...cation/curator-service/api/src/model/automation.ts
...ation/curator-service/api/src/model/date-filter.ts
...urator-service/api/src/clients/aws-batch-client.ts
...rator-service/api/src/clients/aws-lambda-client.ts
...tion/curator-service/api/src/util/assert-string.ts
...ation/curator-service/api/src/controllers/users.ts
...rification/curator-service/api/src/model/origin.ts
...ion/curator-service/api/src/controllers/uploads.ts
...rator-service/api/src/clients/aws-events-client.ts
... and 91 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ffeaba...1defc3d. Read the comment docs.

@iamleeg
Copy link
Contributor Author

iamleeg commented Jul 7, 2021

Great, thanks @sergioloporto!

@sergioloporto
Copy link
Contributor

Great, thanks @sergioloporto!

no worries, please also double check it on your side on Safari and Chrome... a double check is better than one

@iamleeg
Copy link
Contributor Author

iamleeg commented Jul 7, 2021

Yes, works great, thank you! I'm not allowed to approve as it's "my" PR but thumbs up here :)

Copy link
Contributor

@abhidg abhidg left a comment

Choose a reason for hiding this comment

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

Can't test on Safari, but looks good otherwise

@iamleeg iamleeg merged commit f787373 into main Jul 8, 2021
@iamleeg iamleeg deleted the 1976_fix_safari branch July 8, 2021 10:17
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.

4 participants