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

Support Safari on Mac and iOS #2196

Merged
merged 19 commits into from
May 27, 2022
Merged

Support Safari on Mac and iOS #2196

merged 19 commits into from
May 27, 2022

Conversation

filipesilva
Copy link
Collaborator

@filipesilva filipesilva commented May 25, 2022

Followup from #2096 (comment).

This PR removes lookbehind on regexes, and the check that prevents Safari from loading the app.

@vercel
Copy link

vercel bot commented May 25, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
athens ✅ Ready (Inspect) Visit Preview May 26, 2022 at 9:04AM (UTC)

Copy link
Collaborator

@neotyk neotyk left a comment

Choose a reason for hiding this comment

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

LGTM

src/cljs/athens/views/athena.cljs Outdated Show resolved Hide resolved
src/cljs/athens/views/athena.cljs Show resolved Hide resolved
@@ -231,10 +231,17 @@
(utils/parses-to sut/inline-parser->ast

;; Any ASCII punctuation character may be backslash-escaped
;; NOTE: not working in JS environment same as in JVM
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@neotyk would like your input on specialcasing this test. There's another test (a latex one) that is specialcased for cljs too.

I think this isn't a big issue because the hashtag tests already test that word chars before a hashtag will stop it from being a hashtage.

@filipesilva filipesilva changed the title parser lookbehind Support Safari on Mac and iOS May 26, 2022
@@ -1,816 +0,0 @@
(ns athens.cljs-parser-test
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we want to get rid of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I rolled it into the test/athens/parser/impl_test.clj -> impl_test.cljc change, but am not sure if that's the right thing to do. Seemed to be the same tests except for the clj/cljs differences.

@filipesilva filipesilva merged commit 5869f32 into main May 27, 2022
@filipesilva filipesilva deleted the parser-lookbehind branch May 27, 2022 10:58
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