search

ckruse / microformats2-elixir

Microformats2 parser in Elixir
MIT License
20 stars 7 forks source link

Make use of normalized key to atomize keys. #12

Closed jalcine closed 4 years ago

jalcine commented 4 years ago

That makes use of the normalize_key method to conditionally atomize keys. I know there's a potential performance issue with using strings over atoms. However, when I do my templating via Liquid, it helps to have things as strings.

ckruse commented 4 years ago

Hey, cool!

Can I request a change? Using Application.get_env() for configuring libraries is considered an anti pattern in the Elixir world (see https://hexdocs.pm/elixir/master/library-guidelines.html#avoid-application-configuration). This is the reason I removed all options in the current version.

So I'm absolutely not against putting in an option to choose between symbolic and string keys, but I'd rather have it as an option list as an argument to the parser (see the opts example in the linked document).

Can you update your changes to reflect that?

jalcine commented 4 years ago

No problem! I was curious about that too (I actually use that pattern in my indieweb Elixir library - I should move away from it).

(Originally published at: https://v2.jacky.wtf/post/c798efe2-fc6a-4f98-8500-9d44c877d975)

ckruse commented 4 years ago

I implemented this with d11240234aec52e2ae36e7306f1f5b7479d2e8c9

I'd like to make more changes to match the parsing specification again and to fix #4; can I postpone a release until this is finished or do you need this code?