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

Fixed regex patterns for block embed and block ref #1733

Merged

Conversation

sid597
Copy link
Collaborator

@sid597 sid597 commented Oct 14, 2021

@neotyk @filipesilva How do I do this is a better way ?

@vercel
Copy link

vercel bot commented Oct 14, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/athens-research/athens/9eD5eApXaX1gzmP6CKYERd7SYmSg
✅ Preview: https://athens-git-fork-sid597-copy-paste-patterns-fix-athens-research.vercel.app

Copy link
Collaborator

@filipesilva filipesilva left a comment

Choose a reason for hiding this comment

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

I am afraid of this. I think once enough string regex concats are committed into a codebase a portal to hell will appear as a symlink in a toplevel dir. Prior art indicates this is an undesirable situation.

I don't know how to get out of this problem right now. Maybe something like https://github.com/lambdaisland/regal or https://github.com/ztellman/automat would let us slow the descent into madness by providing a higher level language.

@filipesilva filipesilva merged commit be45016 into athensresearch:feature/rtc-v1 Oct 15, 2021
@filipesilva
Copy link
Collaborator

Hm I think I merged this to quickly, and left a carve error because I didn't understand what was happening. Following up on the rtc branch.

@filipesilva
Copy link
Collaborator

@sid597 I ended up reverting the changes on rtc, because as far as I could tell they didn't do anything.

I tried adding these tests in athens.patterns-test:

(deftest block-pattern
  (are [x y] (= (re-find patterns/block-pattern x) y)
    "an ((uid))" "((uid))"
    "not (uid)" nil
    "two ((uid1)) ((uid2))" "((uid1))"))


(deftest block-embed-pattern
  (are [x y] (= (re-find patterns/block-embed-pattern x) y)
    "an {{[[embed]]: ((uid))}}" "{{[[embed]]: ((uid))}}"
    "not {{[[embed]]: abc}}" nil))


(deftest block-refs-pattern
  (are [x y] (= (re-find patterns/block-refs-pattern x) y)
    "an {{[[embed]]: ((uid))}}" "{{[[embed]]: ((uid))}}"
    "an ((uid))" "((uid))"))

The tests passed before and after your changes. Can make some tests for what's supposed to change here, and open a new PR with those changes?

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