drapergem/draper — pull request #319 by Mange (2012-10-25 05:56:53+00:00)
Title: Allow nested decorators

Bug/regression from #173. It was impossible to decorate another decorator, which is the whole point of having generic decorators in the first place.

I assume that it is a bug since this was originally part of the requirements of the fix in #173:

**@steveklabnik wrote**
> We'd have to do this in such a way as to allow arbitrary decoration, without duplicates. Hrm.

---

Since I wanted to be backwards-compatible with the #173 "feature" I left it in, but it's non-trivial to handle the case when one applies the same decorator again (like `A(B(A(object)))`) so I chose to output a warning. More motivations can be found in 9b9f838e2f008b6c191a0ab760a5f0bc6ecb1047.

I know that there's an alternative solution to this and that is to stop using `initialize` for this and instead hook into the class method `new` and/or `decorate`, but I felt that this was too big for me to decide without input from the repo owners. If we did hook into the class methods we could force them to return `input` directly instead of returning a new instance.
Semantically, I would assume `new` always returned a new instance of the class and it up to me not to do something stupid. `decorate`, on the other hand, could definitely do this as its contract could be more akin to "make sure `input` is decorated with this class" instead of "call `new` with `input`".

Here are some other outstanding issues I decided to not fix in this pull request:

* The specs that test the functionality in `initialize` is nested under a describe block called `.decorates` which is something entirely different.
* `Draper::Utils` is marked as private since I don't know if this is a good idea or not. I'd love it if I could access this from my specs as I also get ordering information which is important when applying chains.

Feedback welcome.