Skip to content

Conversation

YoussefHenna
Copy link
Contributor

  • The lookbehind regex is not supported on recent safari versions: https://caniuse.com/js-regexp-lookbehind, it only got support recently.
  • This causes the builder to crash for people that have a slightly older version of safari. Safari also requires mac os upgrades to update, so some users might not be able to update their macos or safari and would therefore never be able to access the builder.
  • I couldn't figure out a regex that does not use the lookbehind, so this is the approach I went with, but let me know if you have other suggestions.

Copy link

@needs needs left a comment

Choose a reason for hiding this comment

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

What about this?

// Should work and test for both `\'` and `\"`.
let nonEscapedQuoteRegex = %re(`/([^\\]|^)["']/g`)

@adnelson
Copy link
Collaborator

Although if @needs suggestion works, that's great too :)

@YoussefHenna
Copy link
Contributor Author

Managed to use regex groups to keep the implementation using regex.

@@ -35,7 +35,7 @@ let parseVJsonWithVariable = parseVariableString => {
regex(inQuotesRegex)
|> map(match => {
match
->Js.String2.replaceByRe(nonEsacapedQuoteRegex, ``)
->Js.String2.replaceByRe(nonEscapedQuoteRegex, `$1`) // First group of the regex is characters before the quote that should be kept
Copy link

Choose a reason for hiding this comment

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

Nice!

@YoussefHenna YoussefHenna merged commit 84ee55e into master Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants