Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Facilitates tag and attribute name processing #116

Closed
wants to merge 3 commits into
from

Conversation

2 participants
Contributor

creynders commented Dec 13, 2013

I didn't update the README yet, since I want to be sure you like the approach.

There's 2 new options: tagNameProcessor and attrNameProcessor. Both accept an Array of functions which are required to have the following signature:

(name) ->
  processedName = process name
  return processedName

I also refactored the normalizeTags functionality to such a processor.
Let me know what you think, then I'll update the README.

Would it be a good idea to provide some of those processors out of the box? For instance stripPrefix and toCamelCase ? As I wrote, actually there is one already: normalizeTags

see #115
see #114

Overall, that seems like a much nicer solution than the previous PR :-) . But isn't processedName returning an array? I believe it should reduce the Array of functions to just a single string.

👍 for refactoring normalizeTags, thats definitely a good idea to base it on this new functionality.

If you like, you could add these processors, I'd probably move them to their own file, so people can see with one look what kind of processors are available.

Contributor

creynders commented Dec 16, 2013

But isn't processedName returning an array?

No, each processor function receives the output of the previous processor function.

you could add these processors, I'd probably move them to their own file

Good idea. I'll try and wrap up the PR today.

creynders added some commits Dec 16, 2013

Finalizes attribute and tag name processing
resolves #116

- updates readme
- adds out-of-the-box processors
- updates tests
Contributor

creynders commented Dec 18, 2013

Not sure whether you get notification when I push commits to a PR, so just a quick FYI: I finished the PR.

Contributor

creynders commented Dec 30, 2013

@Leonidas-from-XIV any idea when you can take a look at this? It'd be nice to get this pulled in. Not wanting to pressure you though 😁

Sorry for leaving that for such a long time, I'll try to check this out in 2014. The only issue I have with this when looking, that the defaults are inserted into the 0.2 defaults object. I think we should have a 0.4 defaults object now that 0.4 is out that probably copies all of 0.2 on runtime and adds the new settings. Something like that but admittedly this is kind of outside the scope of this particular PR.

Owner

Leonidas-from-XIV commented Jan 2, 2014

Merged this pull request finally. Thanks to all contributors and a happy new year!

Contributor

creynders commented Jan 2, 2014

Yeaj! Cake and wodka for everyone!

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