Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Moved backwards_compatibilize into deprecator #1163

Closed
wants to merge 3 commits into from

4 participants

@jpiasetz

I'm on the fence about whether this is a good idea.

Pros

  • Makes Configuration read exactly the current state of jekyll
  • Collects all the backwards compatible code in one place

Cons

  • Adds complexity
@parkr
Owner

Moving code that defines (part of) the Configuration class out of configuration.rb is something I'd like to avoid. I can see this becoming a class method of Deprecator that is then used by Configuration, but I'm not sure. Thanks for the suggestion though.

@jpiasetz jpiasetz closed this
@parkr
Owner

I'd love to get @mattr-'s take on this.

@mattr-
Owner

@jpiasetz A valiant attempt!

To reduce the complexity, I'd implement individual objects for each option that is backwards incompatible and use those objects to remove all the various conditionals, since they all have the same pattern. It would be awesome if you could submit a pull request that implements something like that.

@jpiasetz jpiasetz reopened this
@jpiasetz

@mattr- I'll take a stab at making an object for them.

@jpiasetz jpiasetz referenced this pull request
Closed

Don't do things unless asked #1276

1 of 2 tasks complete
@kelvinst

So @jpiasetz, I'm working in some changes in Deprecator class, for doing this same thing (I didn't see your old pull request)... I'm just documenting the code and will pull request it soon...

@jpiasetz

@kelvinst excellent. I got bog down on how to implement this.

@kelvinst

@mattr-, in #1357 I tried to do what you asking here, can you take a look?

@parkr parkr closed this
@parkr parkr reopened this
@jpiasetz

@mattr- or @parkr should I kill this in lieu of #1357's death?

@mattr-
Owner

For now, I think that's probably best.

@mattr- mattr- closed this
@jpiasetz jpiasetz deleted the jpiasetz:Deprecation branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 29, 2013
  1. @jpiasetz
Commits on Jun 4, 2013
  1. @jpiasetz

    Refactor

    jpiasetz authored
  2. @jpiasetz

    assert_equal is backwards

    jpiasetz authored
This page is out of date. Refresh to see the latest.
View
2  lib/jekyll.rb
@@ -29,8 +29,8 @@ def require_all(path)
# internal requires
require 'jekyll/core_ext'
require 'jekyll/stevenson'
-require 'jekyll/deprecator'
require 'jekyll/configuration'
+require 'jekyll/deprecator'
require 'jekyll/site'
require 'jekyll/convertible'
require 'jekyll/layout'
View
61 lib/jekyll/configuration.rb
@@ -142,66 +142,7 @@ def read_config_files(files)
$stderr.puts "#{err}"
end
- configuration.backwards_compatibilize
- end
-
- # Public: Split a CSV string into an array containing its values
- #
- # csv - the string of comma-separated values
- #
- # Returns an array of the values contained in the CSV
- def csv_to_array(csv)
- csv.split(",").map(&:strip)
- end
-
- # Public: Ensure the proper options are set in the configuration to allow for
- # backwards-compatibility with Jekyll pre-1.0
- #
- # Returns the backwards-compatible configuration
- def backwards_compatibilize
- config = clone
- # Provide backwards-compatibility
- if config.has_key?('auto') || config.has_key?('watch')
- Jekyll::Stevenson.warn "Deprecation:", "Auto-regeneration can no longer" +
- " be set from your configuration file(s). Use the"+
- " --watch/-w command-line option instead."
- config.delete('auto')
- config.delete('watch')
- end
-
- if config.has_key? 'server'
- Jekyll::Stevenson.warn "Deprecation:", "The 'server' configuration option" +
- " is no longer accepted. Use the 'jekyll serve'" +
- " subcommand to serve your site with WEBrick."
- config.delete('server')
- end
-
- if config.has_key? 'server_port'
- Jekyll::Stevenson.warn "Deprecation:", "The 'server_port' configuration option" +
- " has been renamed to 'port'. Please update your config" +
- " file accordingly."
- # copy but don't overwrite:
- config['port'] = config['server_port'] unless config.has_key?('port')
- config.delete('server_port')
- end
-
- if config.has_key?('exclude') && config['exclude'].is_a?(String)
- Jekyll::Stevenson.warn "Deprecation:", "The 'exclude' configuration option" +
- " must now be specified as an array, but you specified" +
- " a string. For now, we've treated the string you provided" +
- " as a list of comma-separated values."
- config['exclude'] = csv_to_array(config['exclude'])
- end
-
- if config.has_key?('include') && config['include'].is_a?(String)
- Jekyll::Stevenson.warn "Deprecation:", "The 'include' configuration option" +
- " must now be specified as an array, but you specified" +
- " a string. For now, we've treated the string you provided" +
- " as a list of comma-separated values."
- config['include'] = csv_to_array(config['include'])
- end
-
- config
+ configuration
end
end
View
49 lib/jekyll/deprecator.rb
@@ -29,4 +29,53 @@ def self.deprecation_message(args, deprecated_argument, message)
end
end
end
+
+ Configuration.class_eval do
+ alias_method "read_config_files_original", "read_config_files"
+
+ # Provide backwards-compatibility
+ def backwards_compatibilize
+ config = clone
+
+ if config.has_key? 'server_port'
+ config['port'] = config['server_port'] unless config.has_key?('port')
+ end
+ deprecate_key('server_post', "has been renamed to 'port'. Please update your config file accordingly.", config)
+ deprecate_key('server', "is no longer accepted. Use the 'jekyll serve' subcommand to serve your site with WEBrick.", config)
+
+ message = "can no longer be set in the configuration file. Use the --watch/-w command-line option instead."
+ deprecate_key('auto', message, config)
+ deprecate_key('watch', message, config)
+
+ config = deprecate_key_array('exclude', config)
+ config = deprecate_key_array('include', config)
+
+ config
+ end
+
+ define_method "read_config_files" do |*files, &block|
+ configuration = send "read_config_files_original", *files, &block
+ configuration.backwards_compatibilize
+ end
+
+ private
+
+ def deprecate_key(name, message, config)
+ if config.has_key? name
+ Jekyll::Stevenson.warn "Deprecation:", "The '" + name + "' configuration option " + message
+ config.delete name
+ end
+ end
+
+ def deprecate_key_array(name, config)
+ if config.has_key?(name) && config[name].is_a?(String)
+ temp = config[name].split(",").map(&:strip)
+ message = "should be an array. The string has been treated as a comma-separated list."
+ deprecate_key(name, message, config)
+ config[name] = temp
+ end
+ config
+ end
+
+ end
end
View
4 test/test_configuration.rb
@@ -71,12 +71,12 @@ class TestConfiguration < Test::Unit::TestCase
should "transform string exclude into an array" do
assert @config.has_key?("exclude")
assert @config.backwards_compatibilize.has_key?("exclude")
- assert_equal @config.backwards_compatibilize["exclude"], %w[READ-ME.md Gemfile CONTRIBUTING.hello.markdown]
+ assert_equal %w[READ-ME.md Gemfile CONTRIBUTING.hello.markdown], @config.backwards_compatibilize["exclude"]
end
should "transform string include into an array" do
assert @config.has_key?("include")
assert @config.backwards_compatibilize.has_key?("include")
- assert_equal @config.backwards_compatibilize["include"], %w[STOP_THE_PRESSES.txt .heloses .git]
+ assert_equal %w[STOP_THE_PRESSES.txt .heloses .git], @config.backwards_compatibilize["include"]
end
end
context "loading configuration" do
Something went wrong with that request. Please try again.