First of all, it's totally great that you're thinking about code style.
I'd like to make changes to both these sections. However, I have a few
issues:
* Could you separate these into two patches, one for the options and
one for the filters?
* I like that red/bluecloth aren't required for unit tests, but that
shouldn't be accomplished by just not testing them. Ideally, if
they exist, they should be tested, but if they don't, it should
output a non-fatal error message.
* I want Haml to throw an exception if a user tries to use one of
these filters when it's not installed. This allows it to show up
on unit tests, be handled without digging through the HTML, be
immediately obvious, etc.
Aside from that, it looks good.
- Nathan
Mislav Marohnić wrote:
> I really love Haml and its beauty, but some of its code really made my
> stomach turn. For instance, so many nasty hacks regarding loading of
> RedCloth and BlueCloth in both filters.rb and unit tests (those are
> even worse) and the over-complex default option handling in
> Haml::Engine#initialize.
>
> I've rewritten the Engine#initialize option handling, as well as
> MarkDown, Textile and RedCloth filters to use lazy loading of
> associated libraries:
> http://pastie.caboo.se/80847 <http://pastie.caboo.se/80847>
>
> Haml was too tightly coupled with these libraries. Ever tried running
> unit tests when you don't have them installed? It's a disaster.
>
> The patch also cleans up the "no_bluecloth/redcloth" require hacks in
> unit tests, replacing them with much more readable
> test_lazy_filter_loading. I've completely removed the
> bluecloth/redcloth bits from the template tests, too, because there is
> no sense for Haml to unit test functionality of 3rd party libraries.
> Haml should just test the integration of its filters, and it does that
> pretty well in test_lazy_filter_loading (and other tests).
>
> Overall:
>
> * BlueCloth and RedCloth are required only when their filter
> classes get instantiated
> * nicer code = maintainability
> * users who don't have redcloth and bluecloth installed can now
> run Haml unit tests!
> * don't unit test Textile/Markdown features
>
> What has changed:
>
> * I've renamed Hash#rec_merge method to deep_merge, since the name
> is nicer and still self-descriptive
> * If you don't have RedCloth or BlueCloth, but use ":textile" or
> ":markdown" filters in Haml templates, this will not raise an
> error anymore! Haml will simply output the unprocessed source in
> the resulting template together with an ERROR notice.
>
> What do you think?
>
> >
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Haml" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/haml?hl=en
-~----------~----~----~----~------~----~------~--~---
