Possible to append to default filter chain? about gollum-lib HOT 5 OPEN

Looked at the code more and realized you currently have to define your custom filters in the Gollum::Filter:: namespace:

It might also be nice to allow implementors to stay out of the Gollum namespace like in my example above?

from gollum-lib.

It might also be nice to allow implementors to stay out of the Gollum namespace like in my example above?

I personally think it's neat to have all filters be part of the same namespace, but if the consensus is this is very counter intuitive I am happy to yield. :) Substantively it would only require modifying the line you quote, I think. Something like:

filter_sym.is_a?(Class) ? fiter_sym.new(self) :  Gollum::Filter.const_get(filter_sym).new(self) 

But my personal feeling is this complicates the code for a minimal gain in enduser-friendliness. Perhaps it's more important that we document clearly how to define a custom filter?

from gollum-lib.

Sorry @beporter, I just realized I never responded to this more verbosely. I do really like the example snippet you give and we would be open to considering a PR that makes something like that possible! If I see things correctly, that functionality would be independent of the namespace issue, no?

Thanks for chipping in regardless!

from gollum-lib.

No worries on timing!

And yes I believe it would remove the current namespacing requirement. EDIT: I may have interpreted this wrong. JUST making the default list a constant would NOT remove the namespace requirement without the additional snippet you suggested. I'm happy to compromise to at least enable the prepend/append functionality.

Quite similarly to you: I'm quite happy to do a PR, but no promises on timing. The nature of open source work is that sometimes it's months between good opportunities to meaningfully contribute. I have a gollum search plugin still sitting on the back burner too. 🤦‍♂️

from gollum-lib.

JUST making the default list a constant would NOT remove the namespace requirement without the additional snippet you suggested. I'm happy to compromise to at least enable the prepend/append functionality.

Right, so there seem to be two options:

1. Add the snippet and be able to do Gollum::Wiki::DEFAULT_FILTERS + [ My::CustomFilter ]
2. Don't add the snippet and only be able to do Gollum::Wiki::DEFAULT_FILTERS + :MyCustomFilter, where MyCustomFilter is under the Gollum::Filter namespace

I'm alright with either approach, 1 means more flexibility for the end user, whereas 2 is less complex in our codebase. My own initial preference was for one, but I'm happy to yield.

By the way, since the order of execution for the filters is important, I guess a typical usage would be something like:

Gollum::Wiki::DEFAULT_FILTERS.insert(Gollum::Wiki::DEFAULT_FILTERS.rindex(:Code), My::CustomFilter)

Maybe we could even come up with a more user friendly solution for that.

And: absolutely understand about the timing! Thanks for taking the time to contribute.

from gollum-lib.