Skip to content
This repository was archived by the owner on Aug 17, 2021. It is now read-only.

improve extractor to better match 'refmt:ed' pxx syntax#116

Merged
dylanirlbeck merged 1 commit into
dylanirlbeck:masterfrom
J-Zeitler:fix/extractor-when-refmt
Jul 13, 2020
Merged

improve extractor to better match 'refmt:ed' pxx syntax#116
dylanirlbeck merged 1 commit into
dylanirlbeck:masterfrom
J-Zeitler:fix/extractor-when-refmt

Conversation

@J-Zeitler

@J-Zeitler J-Zeitler commented Jul 13, 2020

Copy link
Copy Markdown
Contributor

This change enables support for two cases of "refmt":ed pxx syntax as reported in #115

Note: extractor now does not consider the []-brackets normally surrounding the %tw ppx macros

@dylanirlbeck dylanirlbeck left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Overall, the changes LGTM (and thanks for the nicely-written tests!)

I tried to imagine a situation where the regex might incorrectly match something after removing the [ requirement, but I couldn't think of anything. I'll go ahead and merge this in!

@dylanirlbeck dylanirlbeck merged commit ea7e8b2 into dylanirlbeck:master Jul 13, 2020
@dylanirlbeck

Copy link
Copy Markdown
Owner

@all-contributors Please add @J-Zeitler for code

@allcontributors

Copy link
Copy Markdown
Contributor

@dylanirlbeck

I've put up a pull request to add @J-Zeitler! 🎉

@J-Zeitler

Copy link
Copy Markdown
Contributor Author

Cool, I was thinking about how this might break stuff. The important part should be to match %tw as a unique identifier for "tailwind-ppx syntax". If refmt wants to get rid of the hardbrackets, then I guess ppx:es cannot really count on them being part of the ppx-syntax.

(props for the speedy review by the way :) let me know if things start to behave incorrectly)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants