Fix mustache

[dometto]

The mustache-sinatra gem just received a bump release, which now utillizes the latest mustache. Unfortunately, this has suddenly exposed gollum to breaking changes in the latest mustache. I found two:

Broken call to View#escape

Error: NoMethodError private method escape called for <<Precious::Views::Create: ....

Diagnosis: in the latest mustache, views are expected to have an escape instead of an escapeHTML method. We were using the escapeHTML method provided by the Rack::Utils modules. By including that module, we also include a private #escape method, which mustache is now attempting to call.

Fix: instead of including Rack::Utils, just use the default Mustache#escape, which uses CGI.escapeHTML. Overriding the #escape method is only necessary for providing custom (non-HTML) escaping.

Broken template cascades

Our logic for template cascading fails, causing a test failure for using custom partials inside a custom template ("test_overridden_navbar_partial_is_used" in test_template_cascade.rb).

Diagnosis: we provided a custom class method Layout.partial, but the newest version of mustache ended up calling an instance method Page#partial, i.e. the mustache default. So our overriding logic was simply not being called.

Fix: providing the necessary instance method #partial which provides the necessary overriding logic. I have still implemented the helper methods as class methods.

@bartkamphorst I would appreciate a review of this fix, since I'm not really familiar with the template cascade functionality.

This fix would only be included in the next major release of gollum. Should we backport a fix for 5.x, pinning mustache to an older version?

[dometto]

This fix would only be included in the next major release of gollum. Should we backport a fix for 5.x, pinning mustache to an older version?

If this fix is approved, I think we should backport it, because we would then also have a version of gollum compatible with ruby 3.2 👍

[benjaminwil]

I worked on the unit tests for the template cascade feature. I think they describe the feature pretty well if that's helpful.

This may be a good opportunity to add a basic Capybara test for them, too. 👍

[dometto]

This may be a good opportunity to add a basic Capybara test for them, too. 👍

That's a good idea, but I don't see a good way of getting this to work on CI yet, since we're running the tests against a separate gollum instance there. That situation isn't ideal but I don't see a quick way of doing better. Since the normal template_cascade tests picked up the errors here, and since you worked on the tests, I feel this can be merged. :)

Thanks for your feedback!