search

dylanirlbeck / tailwind-ppx

A Reason/OCaml Pre-Processor eXtension (PPX) that validates your Tailwind classes at compile-time.
MIT License
152 stars 15 forks source link

make tail recursive #73

Closed tatchi closed 4 years ago

tatchi commented 4 years ago

List concatenation is not tail-recursive so this change should slightly improve perf.

Not that the order of the produced acceptableClassNames list is not the same anymore. From my understanding, the order does not matter and I verified the tests are passing. So hopefully that's fine :P

dylanirlbeck commented 4 years ago

Hi @tatchi ! Thanks for the PR --- I did not consider that fact, so today I learned something 😄 If you can, can you take a look at #70 and give that PR a review? I think it subsumes your PR/ideas about tail recursion since it also uses the spread operator.

tatchi commented 4 years ago

@dylanirlbeck Sorry I didn't see this PR before making mine. I still see list concatenation so I'll give it a shot and let you know :)

tatchi commented 4 years ago

@dylanirlbeck Ok I think it's good now. I don't know what happened with these images, but my git is constantly trying to transform them from CRLF to LF. That's apparently due to * text eol=lf in the .gitattributes file.

dylanirlbeck commented 4 years ago

Yeah okay that would explain why I was getting the CRLF error as well! Let me remove that from the .gitattributes.

tatchi commented 4 years ago

@dylanirlbeck Should be ok now (hopefully :P)

dylanirlbeck commented 4 years ago

I'm going to pull down your branch and take a look at why the parser is now failing on Windows --- may be related to me removing stuff from the .gitattributes.

tatchi commented 4 years ago

@tatchi Quick question --- are you using refmt for formatting?

I'm not sure, I think it's done via the reason vscode extension.

dylanirlbeck commented 4 years ago

Yeah that's what I thought --- I use ocamlformat for native projects, so I'll create a pre-commit hook so everyone's on the same page.