search

catppuccin / github

🐈‍⬛ Soothing pastel theme for GitHub
MIT License
251 stars 21 forks source link

fix regex #33

Closed rbutera closed 2 years ago

rbutera commented 2 years ago

@andreasgrafen not sure why the latest commit wasn't in the PR before. We need to escape the . in the gists part of the regexp

unseen-ninja commented 2 years ago

Okay, now I might be stupid again, but isn't the . already escaped by the \ in front?

rbutera commented 2 years ago

no, css strings should be double escaped, as explained in my last PR

image

top is the "normal" regex (single escape) that I entered into stylus. This is the pattern that would work in javascript, or regexr, or pretty much anywhere.

bottom is the output regex (double escape) that should be in the css stylesheet

image

pay attention to the text in the red circle

the regex pattern is INSIDE a css string. that means when it is interpreted, the first escape will get 'cancelled' in the process of conversion from string to regex pattern. Therefore, once regex pattern is actually interpreted, if you only started with a single escape, then you end up with no escapes.

the merged PR 'works' because the \. is getting turned into ., ie. "any character". so the current pattern will match gists.github.com and gistsxgithub.com.

It's understandable why it can be quite confusing, because the character we're looking for here is literally the ., but as it is merged it is actually the REGEXP . (any character) not the literal dot.

So yes, it 'works', but it is wrong... and for correctness' sake, it's best to double escape it, as it should be, so that if anyone wants to edit it in the future they don't end up falling into the trap of single escaping.

Hope that makes sense :smile:

unseen-ninja commented 2 years ago

Yeah, it does! Thanks for explaining it to me like I'm five. I'm def. way too tired for all this. :'D