"config" object for setting defaults #41

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@nzaillian

I was using breadcrumbs_on_rails along with a twitter bootstrap-friendly builder (https://gist.github.com/riyad/1933884) and it felt tedious and wrong explicitly passing the "builder" and "separator" options to every "render_breadcrumbs" invocation when these things weren't changing, so I added a Breadcrumbs "config" module member (and supporting logic to the view helpers and SimpleBuilder) so that I could just stick this in an initializer:

# config/initializers/breadcrumbs_on_rails.rb
BreadcrumbsOnRails::Breadcrumbs::config.builder = ::BootstrapBreadcrumbsBuilder
BreadcrumbsOnRails::Breadcrumbs::config.separator = "»"    

Merge if you see fit.

Adding "config" module member (OpenStruct) to
Breadcrumbs module for setting default configuration
options (for example, from a Rails initializer).  Also,
updated view helpers and SimpleBuilder to respect defaults set in the
config object (but giving priority to options explicitly passed in).
@voondo

This comment has been minimized.

Show comment Hide comment
@voondo

voondo Apr 17, 2013

+1

voondo commented Apr 17, 2013

+1

@d-natoli

This comment has been minimized.

Show comment Hide comment
@d-natoli

d-natoli Jun 6, 2013

Contributor

+1

Contributor

d-natoli commented Jun 6, 2013

+1

+ options.delete(:builder) ||
+ Breadcrumbs::config.builder ||
+ Breadcrumbs::SimpleBuilder
+ ).new(self, breadcrumbs, options)

This comment has been minimized.

Show comment Hide comment
@d-natoli

d-natoli Jul 17, 2013

Contributor

Why not make Breadcrumbs::config.builder equal Breadcrumbs::SimpleBuilder by default? That way you wouldn't have to have the double ors here, it'd just become

builder = (options.delete(:builder) || Breadcrumbs::config.builder).new(self, breadcrumbs, options)
@d-natoli

d-natoli Jul 17, 2013

Contributor

Why not make Breadcrumbs::config.builder equal Breadcrumbs::SimpleBuilder by default? That way you wouldn't have to have the double ors here, it'd just become

builder = (options.delete(:builder) || Breadcrumbs::config.builder).new(self, breadcrumbs, options)

This comment has been minimized.

Show comment Hide comment
@weppos

weppos Jan 25, 2014

Owner

I agree.

@weppos

weppos Jan 25, 2014

Owner

I agree.

@@ -81,7 +87,7 @@ class SimpleBuilder < Builder
def render
@elements.collect do |element|
render_element(element)
- end.join(@options[:separator] || " &raquo; ")
+ end.join(@options[:separator] || Breadcrumbs::config.separator || " &raquo; ")

This comment has been minimized.

Show comment Hide comment
@d-natoli

d-natoli Jul 17, 2013

Contributor

Same here, just default Breadcrumbs::config.separator to " » ".

@d-natoli

d-natoli Jul 17, 2013

Contributor

Same here, just default Breadcrumbs::config.separator to " » ".

@weppos

This comment has been minimized.

Show comment Hide comment
@weppos

weppos Jan 25, 2014

Owner

Wow, it took almost 1 year to check this PR. Sorry for the long time.

I'm not a big fan of using OpenStruct in this case. In fact, the set of defined options should be constrained to prevent typos or simply passing anything into the config object.

Moreover, I would prefer Breadcrumbs::config to be referenced as Breadcrumbs::config.

Here's a possible implementation

class BreadcrumbsOnRails
  class Configuration
    attr_accessor :default_separator, :default_builder

    def initialize
      self.default_separator = " &raquo; "
      self.default_builder = Breadcrumbs::SimpleBuilder
    end
  end

  class << self
    def configure
      @config ||= Configuration.new
      block_given? ? yield(@config) : @config
    end
    alias :config :configure
  end
end

This will make possible to write

BreadcrumbsOnRails.configure do |config|
  config.default_separator = ' ... '
end
Owner

weppos commented Jan 25, 2014

Wow, it took almost 1 year to check this PR. Sorry for the long time.

I'm not a big fan of using OpenStruct in this case. In fact, the set of defined options should be constrained to prevent typos or simply passing anything into the config object.

Moreover, I would prefer Breadcrumbs::config to be referenced as Breadcrumbs::config.

Here's a possible implementation

class BreadcrumbsOnRails
  class Configuration
    attr_accessor :default_separator, :default_builder

    def initialize
      self.default_separator = " &raquo; "
      self.default_builder = Breadcrumbs::SimpleBuilder
    end
  end

  class << self
    def configure
      @config ||= Configuration.new
      block_given? ? yield(@config) : @config
    end
    alias :config :configure
  end
end

This will make possible to write

BreadcrumbsOnRails.configure do |config|
  config.default_separator = ' ... '
end

@ghost ghost assigned weppos Jan 25, 2014

@weppos

This comment has been minimized.

Show comment Hide comment
@weppos

weppos Jan 18, 2017

Owner

Closing as very old, and missing tests. Further customizations can be accomplished with a custom builder.

Owner

weppos commented Jan 18, 2017

Closing as very old, and missing tests. Further customizations can be accomplished with a custom builder.

@weppos weppos closed this Jan 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment