Sinatra::Namespace doesn't respect some settings? #28

Closed
chanks opened this Issue Dec 31, 2011 · 9 comments

Projects

None yet

8 participants

@chanks
chanks commented Dec 31, 2011

In my main configure block I use:

set :haml, :format => :html5, :ugly => true

When I use the 'haml' helper to render a page for a normal (non-namespaced) route handler, it appears as I'd expect - with the HTML5 doctype and the ugly formatting. But when I use the 'haml' helper in a namespaced route handler, it appears that neither of these options are being respected.

I can work around this by doing haml :template, :format => :html5, :ugly => true in every namespaced route handler, which isn't exactly clean. I've tried using a set :haml line with the same options inside my namespace, but then I get a "may not set haml" error.

@chanks
chanks commented Dec 31, 2011

Sorry, I should have mentioned that I'm using sinatra 1.3.2 and sinatra-contrib 1.3.1.

@pieter
pieter commented Jan 16, 2012

I have the same issue with erubis' escape_html

@federomero

Here's a simple gist that shows the root of the problem.

https://gist.github.com/3154659

When using namespaces settings.respond_to? is return false on values that have been set.

@reconbot

I don't think settings support is fully baked beyond settings :views

https://github.com/sinatra/sinatra-contrib/blob/master/lib/sinatra/namespace.rb#L201

        raise ArgumentError, "may not set #{key}" if key != :views

But maybe we can work this out? The settings object is the namespace and not the application

https://github.com/sinatra/sinatra-contrib/blob/master/lib/sinatra/namespace.rb#L131

module InstanceMethods
  def settings
    @namespace
  end

My monkeypatch doesn't work - I think I'm missing how namespace works.

module Sinatra
  module Namespace
    module InstanceMethods
      def settings
        super.settings
      end
    end
  end
end
@lokikl
lokikl commented Jun 21, 2013

Monkey patching #settings make my set :erb, :trim => '-' works as usual under namespace. Please comment if there is a correct approach.

class MyApp < Sinatra::Base
  namespace '/ns' do
    helpers do
      def settings
        MyApp.settings
      end
    end
  end
end
@zzak
Member
zzak commented Aug 19, 2013

I cannot reproduce this with the following example:

require 'sinatra'
require 'sinatra/namespace'

set :foo, 42

namespace '/foo' do
  get '/bar' do
    settings.foo.to_s
  end
end
@zzak zzak added a commit that referenced this issue Aug 19, 2013
@zzak zzak specs for sinatra/sinatra-contrib#28 4fedd2f
@federomero

That works correctly, the problem is that settings.respond_to?(:foo) returns false instead of true and that's the root of the original issue.

@lest lest added a commit to lest/sinatra-contrib that referenced this issue Jun 2, 2014
@lest lest allow checking setting existance with respond_to?
closes #28
de0c0d4
@zzak zzak added a commit that referenced this issue Sep 23, 2014
@zzak @zzak zzak + zzak specs for sinatra/sinatra-contrib#28 1dbcec9
@lest lest added a commit to lest/sinatra-contrib that referenced this issue Nov 25, 2014
@lest lest allow checking setting existance with respond_to?
closes #28
1ae98c9
@lest lest added a commit to lest/sinatra-contrib that referenced this issue Nov 25, 2014
@lest lest allow checking setting existance with respond_to?
closes #28
27ee8c5
@ginjo
ginjo commented Jan 27, 2015

This is still an issue in January 2015 with sinatra 1.4.5 and sinatra-contrib 1.4.2.
Is there a correct way to allow namespace access to application settings (and functionality that is dependent on application settings, like templates etc)?

If not, is there a future path of likely resolution, so we can at least structure our applications in a way that will accommodate the eventual solution?

@pedro
pedro commented May 12, 2015

Just ran into this too – sinatra 1.4.6, sinatra-contrib 1.4.2:

Here's a very simple way to reproduce, extending on @zzak's:

require 'sinatra'
require 'sinatra/namespace'

set :foo, 42

get "/test" do
  settings.respond_to?(:foo).to_s
end

namespace '/namespace' do
  get '/test' do
    settings.respond_to?(:foo).to_s
  end
end

The monkeypatch suggested by @reconbot worked for me:

module Sinatra
  module Namespace
    module InstanceMethods
      def settings
        super.settings
      end
    end
  end
end

I'd send a pull, but not aware of the context here and why settings is being defined this way in first place. Is that only to support a different folder for views?

@zzak zzak closed this in #139 May 22, 2015
@zzak zzak added a commit to zzak/sinatra-contrib that referenced this issue Jul 22, 2016
@zzak @zzak zzak + zzak specs for sinatra/sinatra-contrib#28 6181515
@zzak zzak pushed a commit to zzak/sinatra-contrib that referenced this issue Jul 22, 2016
@lest lest allow checking setting existance with respond_to?
closes #28
40dbfc9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment