From e6a924822c0efbb8414aa326c67ef0a396bd17c2 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 16 May 2024 11:27:35 +0200 Subject: [PATCH 1/4] Remove OffsetAwareRuleSet and make RuleSet take keyword arguments Deprecation message look like: /Users/simon/Downloads/css_parser/test/test_css_parser_basic.rb:75: warning: [DEPRECATION] positional arguments are deprecated use keyword instead. --- CHANGELOG.md | 3 ++ Rakefile | 6 ++- lib/css_parser/parser.rb | 14 ++++-- lib/css_parser/rule_set.rb | 51 ++++++++++++++------- test/test_css_parser_basic.rb | 8 ++-- test/test_css_parser_misc.rb | 6 +-- test/test_merging.rb | 54 +++++++++++------------ test/test_rule_set.rb | 28 ++++++------ test/test_rule_set_expanding_shorthand.rb | 2 +- 9 files changed, 104 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1448ae2..16fa114 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ### Unreleased +* RuleSet initialize now takes keyword argument, positional arguments are still supported but deprecated +* Removed OffsetAwareRuleSet, it's a RuleSet with optional attributes filename and offset + ### Version v1.18.0 * Drop Ruby 2.7 compatibility for parity with Premailer [#149](https://github.com/premailer/css_parser/pull/149) diff --git a/Rakefile b/Rakefile index 021cd23..4ae5610 100644 --- a/Rakefile +++ b/Rakefile @@ -13,7 +13,11 @@ Rake::TestTask.new do |test| test.verbose = true end -RuboCop::RakeTask.new +RuboCop::RakeTask.new do |t| + # allow you to run "$ rake rubocop -a" to autofix + t.options << '-a' if ARGV.include?('-a') + t.options << '-A' if ARGV.include?('-A') +end desc 'Run a performance evaluation.' task :benchmark do diff --git a/lib/css_parser/parser.rb b/lib/css_parser/parser.rb index 609b0ac..d8385f8 100644 --- a/lib/css_parser/parser.rb +++ b/lib/css_parser/parser.rb @@ -166,9 +166,13 @@ def add_block!(block, options = {}) # Add a CSS rule by setting the +selectors+, +declarations+ and +media_types+. # - # +media_types+ can be a symbol or an array of symbols. + # +media_types+ can be a symbol or an array of symbols. default to :all + # optional fields for source location for source location + # +filename+ can be a string or uri pointing to the file or url location. + # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. + def add_rule!(selectors, declarations, media_types = :all) - rule_set = RuleSet.new(selectors, declarations) + rule_set = RuleSet.new(selectors: selectors, block: declarations) add_rule_set!(rule_set, media_types) rescue ArgumentError => e raise e if @options[:rule_set_exceptions] @@ -180,7 +184,11 @@ def add_rule!(selectors, declarations, media_types = :all) # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. # +media_types+ can be a symbol or an array of symbols. def add_rule_with_offsets!(selectors, declarations, filename, offset, media_types = :all) - rule_set = OffsetAwareRuleSet.new(filename, offset, selectors, declarations) + rule_set = RuleSet.new( + selectors: selectors, block: declarations, + offset: offset, filename: filename + ) + add_rule_set!(rule_set, media_types) end diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index c3904c1..d423b97 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -223,6 +223,12 @@ def normalize_property(property) extend Forwardable + # optional field for storing source reference + # File offset range + attr_reader :offset + # the local or remote location + attr_accessor :filename + # Array of selector strings. attr_reader :selectors @@ -237,9 +243,38 @@ def normalize_property(property) alias []= add_declaration! alias remove_declaration! delete - def initialize(selectors, block, specificity = nil) + def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, specificity: nil) # rubocop:disable Metrics/ParameterLists + if args.any? + if selectors || block || offset || filename || specificity + raise ArgumentError, "don't mix positional and keyword arguments" + end + + warn '[DEPRECATION] positional arguments are deprecated use keyword instead.', uplevel: 1 + + case args.length + when 2 + selectors, block = args + when 3 + selectors, block, specificity = args + when 4 + filename, offset, selectors, block = args + when 5 + filename, offset, selectors, block, specificity = args + else + raise ArgumentError + end + end + @selectors = [] @specificity = specificity + + unless offset.nil? == filename.nil? + raise ArgumentError, 'require both offset and filename' + end + + @offset = offset + @filename = filename + parse_selectors!(selectors) if selectors parse_declarations!(block) end @@ -650,18 +685,4 @@ def split_value_preserving_function_whitespace(value) end end end - - class OffsetAwareRuleSet < RuleSet - # File offset range - attr_reader :offset - - # the local or remote location - attr_accessor :filename - - def initialize(filename, offset, selectors, block, specificity = nil) - super(selectors, block, specificity) - @offset = offset - @filename = filename - end - end end diff --git a/test/test_css_parser_basic.rb b/test/test_css_parser_basic.rb index c5c341c..1ca8141 100644 --- a/test/test_css_parser_basic.rb +++ b/test/test_css_parser_basic.rb @@ -40,15 +40,15 @@ def test_adding_a_rule end def test_adding_a_rule_set - rs = CssParser::RuleSet.new('div', 'color: blue;') + rs = CssParser::RuleSet.new(selectors: 'div', block: 'color: blue;') @cp.add_rule_set!(rs) assert_equal 'color: blue;', @cp.find_by_selector('div').join(' ') end def test_removing_a_rule_set - rs = CssParser::RuleSet.new('div', 'color: blue;') + rs = CssParser::RuleSet.new(selectors: 'div', block: 'color: blue;') @cp.add_rule_set!(rs) - rs2 = CssParser::RuleSet.new('div', 'color: blue;') + rs2 = CssParser::RuleSet.new(selectors: 'div', block: 'color: blue;') @cp.remove_rule_set!(rs2) assert_equal '', @cp.find_by_selector('div').join(' ') end @@ -72,7 +72,7 @@ def test_toggling_uri_conversion end def test_converting_to_hash - rs = CssParser::RuleSet.new('div', 'color: blue;') + rs = CssParser::RuleSet.new(selectors: 'div', block: 'color: blue;') @cp.add_rule_set!(rs) hash = @cp.to_h assert_equal 'blue', hash['all']['div']['color'] diff --git a/test/test_css_parser_misc.rb b/test/test_css_parser_misc.rb index 758355a..cd88118 100644 --- a/test/test_css_parser_misc.rb +++ b/test/test_css_parser_misc.rb @@ -195,20 +195,20 @@ def test_ruleset_with_braces # rulesets = [] # # parser['div'].each do |declaration| - # rulesets << RuleSet.new('div', declaration) + # rulesets << RuleSet.new(selectors: 'div', block: declaration) # end # # merged = CssParser.merge(rulesets) # # result: # merged.to_s => "{ background-color: black !important; }" - new_rule = RuleSet.new('div', "{ background-color: black !important; }") + new_rule = RuleSet.new(selectors: 'div', block: "{ background-color: black !important; }") assert_equal 'div { background-color: black !important; }', new_rule.to_s end def test_content_with_data - rule = RuleSet.new('div', '{content: url(data:image/png;base64,LOTSOFSTUFF)}') + rule = RuleSet.new(selectors: 'div', block: '{content: url(data:image/png;base64,LOTSOFSTUFF)}') assert_includes rule.to_s, "image/png;base64,LOTSOFSTUFF" end diff --git a/test/test_merging.rb b/test/test_merging.rb index a6a8db9..360f2ae 100644 --- a/test/test_merging.rb +++ b/test/test_merging.rb @@ -10,16 +10,16 @@ def setup end def test_simple_merge - rs1 = RuleSet.new(nil, 'color: black;') - rs2 = RuleSet.new(nil, 'margin: 0px;') + rs1 = RuleSet.new(block: 'color: black;') + rs2 = RuleSet.new(block: 'margin: 0px;') merged = CssParser.merge(rs1, rs2) assert_equal '0px;', merged['margin'] assert_equal 'black;', merged['color'] end def test_merging_array - rs1 = RuleSet.new(nil, 'color: black;') - rs2 = RuleSet.new(nil, 'margin: 0px;') + rs1 = RuleSet.new(block: 'color: black;') + rs2 = RuleSet.new(block: 'margin: 0px;') merged = CssParser.merge([rs1, rs2]) assert_equal '0px;', merged['margin'] assert_equal 'black;', merged['color'] @@ -41,51 +41,51 @@ def test_merging_with_compound_selectors end def test_merging_multiple - rs1 = RuleSet.new(nil, 'color: black;') - rs2 = RuleSet.new(nil, 'margin: 0px;') - rs3 = RuleSet.new(nil, 'margin: 5px;') + rs1 = RuleSet.new(block: 'color: black;') + rs2 = RuleSet.new(block: 'margin: 0px;') + rs3 = RuleSet.new(block: 'margin: 5px;') merged = CssParser.merge(rs1, rs2, rs3) assert_equal '5px;', merged['margin'] end def test_multiple_selectors_should_have_proper_specificity - rs1 = RuleSet.new('p, a[rel="external"]', 'color: black;') - rs2 = RuleSet.new('a', 'color: blue;') + rs1 = RuleSet.new(selectors: 'p, a[rel="external"]', block: 'color: black;') + rs2 = RuleSet.new(selectors: 'a', block: 'color: blue;') merged = CssParser.merge(rs1, rs2) assert_equal 'black;', merged['color'] end def test_setting_specificity - rs1 = RuleSet.new(nil, 'color: red;', 20) - rs2 = RuleSet.new(nil, 'color: blue;', 10) + rs1 = RuleSet.new(block: 'color: red;', specificity: 20) + rs2 = RuleSet.new(block: 'color: blue;', specificity: 10) merged = CssParser.merge(rs1, rs2) assert_equal 'red;', merged['color'] end def test_properties_should_be_case_insensitive - rs1 = RuleSet.new(nil, ' CoLor : red ;', 20) - rs2 = RuleSet.new(nil, 'color: blue;', 10) + rs1 = RuleSet.new(block: ' CoLor : red ;', specificity: 20) + rs2 = RuleSet.new(block: 'color: blue;', specificity: 10) merged = CssParser.merge(rs1, rs2) assert_equal 'red;', merged['color'] end def test_merging_backgrounds - rs1 = RuleSet.new(nil, 'background-color: black;') - rs2 = RuleSet.new(nil, 'background-image: none;') + rs1 = RuleSet.new(block: 'background-color: black;') + rs2 = RuleSet.new(block: 'background-image: none;') merged = CssParser.merge(rs1, rs2) assert_equal 'black none;', merged['background'] end def test_merging_dimensions - rs1 = RuleSet.new(nil, 'margin: 3em;') - rs2 = RuleSet.new(nil, 'margin-left: 1em;') + rs1 = RuleSet.new(block: 'margin: 3em;') + rs2 = RuleSet.new(block: 'margin-left: 1em;') merged = CssParser.merge(rs1, rs2) assert_equal '3em 3em 3em 1em;', merged['margin'] end def test_merging_fonts - rs1 = RuleSet.new(nil, 'font: 11px Arial;') - rs2 = RuleSet.new(nil, 'font-weight: bold;') + rs1 = RuleSet.new(block: 'font: 11px Arial;') + rs2 = RuleSet.new(block: 'font-weight: bold;') merged = CssParser.merge(rs1, rs2) assert_equal 'bold 11px Arial;', merged['font'] end @@ -97,32 +97,32 @@ def test_raising_error_on_bad_type end def test_returning_early_with_only_one_params - rs = RuleSet.new(nil, 'font-weight: bold;') + rs = RuleSet.new(block: 'font-weight: bold;') merged = CssParser.merge(rs) assert_equal rs.object_id, merged.object_id end def test_merging_important - rs1 = RuleSet.new(nil, 'color: black !important;') - rs2 = RuleSet.new(nil, 'color: red;') + rs1 = RuleSet.new(block: 'color: black !important;') + rs2 = RuleSet.new(block: 'color: red;') merged = CssParser.merge(rs1, rs2) assert_equal 'black !important;', merged['color'] end def test_merging_multiple_important - rs1 = RuleSet.new(nil, 'color: black !important;', 1000) - rs2 = RuleSet.new(nil, 'color: red !important;', 1) + rs1 = RuleSet.new(block: 'color: black !important;', specificity: 1000) + rs2 = RuleSet.new(block: 'color: red !important;', specificity: 1) merged = CssParser.merge(rs1, rs2) assert_equal 'black !important;', merged['color'] - rs3 = RuleSet.new(nil, 'color: blue !important;', 1000) + rs3 = RuleSet.new(block: 'color: blue !important;', specificity: 1000) merged = CssParser.merge(rs1, rs2, rs3) assert_equal 'blue !important;', merged['color'] end def test_merging_shorthand_important - rs1 = RuleSet.new(nil, 'background: black none !important;') - rs2 = RuleSet.new(nil, 'background-color: red;') + rs1 = RuleSet.new(block: 'background: black none !important;') + rs2 = RuleSet.new(block: 'background-color: red;') merged = CssParser.merge(rs1, rs2) assert_equal 'black !important;', merged['background-color'] end diff --git a/test/test_rule_set.rb b/test/test_rule_set.rb index e8bf458..e4bfd5b 100644 --- a/test/test_rule_set.rb +++ b/test/test_rule_set.rb @@ -12,7 +12,7 @@ def setup end def test_setting_property_values - rs = RuleSet.new(nil, nil) + rs = RuleSet.new rs['background-color'] = 'red' assert_equal('red;', rs['background-color']) @@ -22,12 +22,12 @@ def test_setting_property_values end def test_getting_property_values - rs = RuleSet.new('#content p, a', 'color: #fff;') + rs = RuleSet.new(selectors: '#content p, a', block: 'color: #fff;') assert_equal('#fff;', rs['color']) end def test_getting_property_value_ignoring_case - rs = RuleSet.new('#content p, a', 'color: #fff;') + rs = RuleSet.new(selectors: '#content p, a', block: 'color: #fff;') assert_equal('#fff;', rs[' ColoR ']) end @@ -38,7 +38,7 @@ def test_each_selector ] actual = [] - rs = RuleSet.new('#content p, a', 'color: #fff;') + rs = RuleSet.new(selectors: '#content p, a', block: 'color: #fff;') rs.each_selector do |sel, decs, spec| actual << {selector: sel, declarations: decs, specificity: spec} end @@ -54,7 +54,7 @@ def test_each_declaration ] actual = Set.new - rs = RuleSet.new(nil, 'color: #fff; Background: white none no-repeat !important; margin: 1px -0.25em;') + rs = RuleSet.new(block: 'color: #fff; Background: white none no-repeat !important; margin: 1px -0.25em;') rs.each_declaration do |prop, val, imp| actual << {property: prop, value: val, is_important: imp} end @@ -64,7 +64,7 @@ def test_each_declaration def test_each_declaration_respects_order css_fragment = "margin: 0; padding: 20px; margin-bottom: 28px;" - rs = RuleSet.new(nil, css_fragment) + rs = RuleSet.new(block: css_fragment) expected = %w[margin padding margin-bottom] actual = [] rs.each_declaration { |prop, _val, _imp| actual << prop } @@ -72,38 +72,38 @@ def test_each_declaration_respects_order end def test_each_declaration_containing_semicolons - rs = RuleSet.new(nil, "background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABwAAAAiCAMAAAB7);" \ - "background-repeat: no-repeat") + rs = RuleSet.new(block: "background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABwAAAAiCAMAAAB7);" \ + "background-repeat: no-repeat") assert_equal('url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABwAAAAiCAMAAAB7);', rs['background-image']) assert_equal('no-repeat;', rs['background-repeat']) end def test_selector_sanitization selectors = "h1, h2,\nh3 " - rs = RuleSet.new(selectors, "color: #fff;") + rs = RuleSet.new(selectors: selectors, block: "color: #fff;") assert rs.selectors.member?("h3") end def test_multiple_selectors_to_s selectors = "#content p, a" - rs = RuleSet.new(selectors, "color: #fff;") + rs = RuleSet.new(selectors: selectors, block: "color: #fff;") assert_match(/^\s*#content p,\s*a\s*\{/, rs.to_s) end def test_declarations_to_s declarations = 'color: #fff; font-weight: bold;' - rs = RuleSet.new('#content p, a', declarations) + rs = RuleSet.new(selectors: '#content p, a', block: declarations) assert_equal(declarations.split.sort, rs.declarations_to_s.split.sort) end def test_important_declarations_to_s declarations = 'color: #fff; font-weight: bold !important;' - rs = RuleSet.new('#content p, a', declarations) + rs = RuleSet.new(selectors: '#content p, a', block: declarations) assert_equal(declarations.split.sort, rs.declarations_to_s.split.sort) end def test_overriding_specificity - rs = RuleSet.new('#content p, a', 'color: white', 1000) + rs = RuleSet.new(selectors: '#content p, a', block: 'color: white', specificity: 1000) rs.each_selector do |_sel, _decs, spec| assert_equal 1000, spec end @@ -112,7 +112,7 @@ def test_overriding_specificity def test_not_raised_issue68 ok = true begin - RuleSet.new('td', 'border-top: 5px solid; border-color: #fffff0;') + RuleSet.new(selectors: 'td', block: 'border-top: 5px solid; border-color: #fffff0;') rescue ok = false end diff --git a/test/test_rule_set_expanding_shorthand.rb b/test/test_rule_set_expanding_shorthand.rb index d7b3887..ea7b719 100644 --- a/test/test_rule_set_expanding_shorthand.rb +++ b/test/test_rule_set_expanding_shorthand.rb @@ -347,7 +347,7 @@ def test_functions_with_commas protected def expand_declarations(declarations) - ruleset = RuleSet.new(nil, declarations) + ruleset = RuleSet.new(block: declarations) ruleset.expand_shorthand! collected = {} From 9c1128aca45bb57b44223dc873064acbca5e1e2d Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 16 May 2024 11:47:37 +0200 Subject: [PATCH 2/4] Deprecate `add_rule!` and `add_rule_with_offsets!` for `append_rule!` --- CHANGELOG.md | 1 + lib/css_parser/parser.rb | 33 +++++++++++++++++++++++++-------- test/test_css_parser_loading.rb | 12 ------------ test/test_css_parser_misc.rb | 24 ++++++++++++++++++++++++ 4 files changed, 50 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16fa114..3c6c14c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Unreleased +* Deprecate `add_rule!` (positional arguments)and `add_rule_with_offsets!` for `add_rule!` (keyword argument) * RuleSet initialize now takes keyword argument, positional arguments are still supported but deprecated * Removed OffsetAwareRuleSet, it's a RuleSet with optional attributes filename and offset diff --git a/lib/css_parser/parser.rb b/lib/css_parser/parser.rb index d8385f8..3f18515 100644 --- a/lib/css_parser/parser.rb +++ b/lib/css_parser/parser.rb @@ -164,32 +164,49 @@ def add_block!(block, options = {}) parse_block_into_rule_sets!(block, options) end - # Add a CSS rule by setting the +selectors+, +declarations+ and +media_types+. + # Add a CSS rule by setting the +selectors+, +declarations+ + # and +media_types+. Optional pass +filename+ , +offset+ for source + # reference too. # # +media_types+ can be a symbol or an array of symbols. default to :all # optional fields for source location for source location # +filename+ can be a string or uri pointing to the file or url location. # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. + def append_rule!(selectors: nil, block: nil, filename: nil, offset: nil, media_types: :all) + rule_set = RuleSet.new( + selectors: selectors, block: block, + offset: offset, filename: filename + ) - def add_rule!(selectors, declarations, media_types = :all) - rule_set = RuleSet.new(selectors: selectors, block: declarations) add_rule_set!(rule_set, media_types) rescue ArgumentError => e raise e if @options[:rule_set_exceptions] end + # Add a CSS rule by setting the +selectors+, +declarations+ and +media_types+. + # + # +media_types+ can be a symbol or an array of symbols. default to :all + # optional fields for source location for source location + # +filename+ can be a string or uri pointing to the file or url location. + # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. + + def add_rule!(selectors, declarations, media_types = :all) + warn '[DEPRECATION] `add_rule!` is deprecated. Please use `append_rule!` instead.', uplevel: 1 + + append_rule!(selectors: selectors, block: declarations, media_types: media_types) + end + # Add a CSS rule by setting the +selectors+, +declarations+, +filename+, +offset+ and +media_types+. # # +filename+ can be a string or uri pointing to the file or url location. # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. # +media_types+ can be a symbol or an array of symbols. def add_rule_with_offsets!(selectors, declarations, filename, offset, media_types = :all) - rule_set = RuleSet.new( - selectors: selectors, block: declarations, - offset: offset, filename: filename + warn '[DEPRECATION] `add_rule_with_offsets!` is deprecated. Please use `append_rule!` instead.', uplevel: 1 + append_rule!( + selectors: selectors, block: declarations, media_types: media_types, + filename: filename, offset: offset ) - - add_rule_set!(rule_set, media_types) end # Add a CssParser RuleSet object. diff --git a/test/test_css_parser_loading.rb b/test/test_css_parser_loading.rb index f1ee7c9..b8c8457 100644 --- a/test/test_css_parser_loading.rb +++ b/test/test_css_parser_loading.rb @@ -204,16 +204,4 @@ def test_toggling_not_found_exceptions cp_without_exceptions.load_uri!("#{@uri_base}/no-exist.xyz") end - - def test_rule_set_argument_exceptions - cp_with_exceptions = Parser.new(rule_set_exceptions: true) - - assert_raises ArgumentError, 'background-color value is empty' do - cp_with_exceptions.add_rule!('body', 'background-color: !important') - end - - cp_without_exceptions = Parser.new(rule_set_exceptions: false) - - cp_without_exceptions.add_rule!('body', 'background-color: !important') - end end diff --git a/test/test_css_parser_misc.rb b/test/test_css_parser_misc.rb index cd88118..bc7d17d 100644 --- a/test/test_css_parser_misc.rb +++ b/test/test_css_parser_misc.rb @@ -226,4 +226,28 @@ def test_enumerator_nonempty assert_equal 'color: black;', desc end end + + def test_catching_argument_exceptions_for_add_rule + cp_with_exceptions = Parser.new(rule_set_exceptions: true) + + assert_raises ArgumentError, 'background-color value is empty' do + cp_with_exceptions.add_rule!('body', 'background-color: !important') + end + + cp_without_exceptions = Parser.new(rule_set_exceptions: false) + + cp_without_exceptions.add_rule!('body', 'background-color: !important') + end + + def test_catching_argument_exceptions_for_add_rule_with_offsets + cp_with_exceptions = Parser.new(capture_offsets: true, rule_set_exceptions: true) + + assert_raises ArgumentError, 'background-color value is empty' do + cp_with_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1) + end + + cp_without_exceptions = Parser.new(capture_offsets: true, rule_set_exceptions: false) + + cp_without_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1) + end end From e3b48067dbf6d3b6a5db87fecb49429891cd2942 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 16 May 2024 13:17:15 +0200 Subject: [PATCH 3/4] Unify flow for code flow for adding rule --- lib/css_parser/parser.rb | 70 ++++++++++++++---------- lib/css_parser/rule_set.rb | 2 +- test/test_css_parser_basic.rb | 2 +- test/test_css_parser_media_types.rb | 10 ++-- test/test_css_parser_misc.rb | 32 +++++++++-- test/test_rule_set_creating_shorthand.rb | 8 ++- 6 files changed, 81 insertions(+), 43 deletions(-) diff --git a/lib/css_parser/parser.rb b/lib/css_parser/parser.rb index 3f18515..0f7c745 100644 --- a/lib/css_parser/parser.rb +++ b/lib/css_parser/parser.rb @@ -172,28 +172,36 @@ def add_block!(block, options = {}) # optional fields for source location for source location # +filename+ can be a string or uri pointing to the file or url location. # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. - def append_rule!(selectors: nil, block: nil, filename: nil, offset: nil, media_types: :all) - rule_set = RuleSet.new( - selectors: selectors, block: block, - offset: offset, filename: filename - ) + def add_rule!(*args, selectors: nil, block: nil, filename: nil, offset: nil, media_types: :all) # rubocop:disable Metrics/ParameterLists + if args.any? + media_types = nil + if selectors || block || filename || offset || media_types + raise ArgumentError, "don't mix positional and keyword arguments arguments" + end - add_rule_set!(rule_set, media_types) - rescue ArgumentError => e - raise e if @options[:rule_set_exceptions] - end + warn '[DEPRECATION] `add_rule!` with positional arguments is deprecated. ' \ + 'Please use keyword arguments instead.', uplevel: 1 - # Add a CSS rule by setting the +selectors+, +declarations+ and +media_types+. - # - # +media_types+ can be a symbol or an array of symbols. default to :all - # optional fields for source location for source location - # +filename+ can be a string or uri pointing to the file or url location. - # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. - - def add_rule!(selectors, declarations, media_types = :all) - warn '[DEPRECATION] `add_rule!` is deprecated. Please use `append_rule!` instead.', uplevel: 1 + case args.length + when 2 + selectors, block = args + when 3 + selectors, block, media_types = args + else + raise ArgumentError + end + end - append_rule!(selectors: selectors, block: declarations, media_types: media_types) + begin + rule_set = RuleSet.new( + selectors: selectors, block: block, + offset: offset, filename: filename + ) + + add_rule_set!(rule_set, media_types) + rescue ArgumentError => e + raise e if @options[:rule_set_exceptions] + end end # Add a CSS rule by setting the +selectors+, +declarations+, +filename+, +offset+ and +media_types+. @@ -202,8 +210,8 @@ def add_rule!(selectors, declarations, media_types = :all) # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. # +media_types+ can be a symbol or an array of symbols. def add_rule_with_offsets!(selectors, declarations, filename, offset, media_types = :all) - warn '[DEPRECATION] `add_rule_with_offsets!` is deprecated. Please use `append_rule!` instead.', uplevel: 1 - append_rule!( + warn '[DEPRECATION] `add_rule_with_offsets!` is deprecated. Please use `add_rule!` instead.', uplevel: 1 + add_rule!( selectors: selectors, block: declarations, media_types: media_types, filename: filename, offset: offset ) @@ -385,11 +393,14 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc: current_declarations.strip! unless current_declarations.empty? + add_rule_options = { + selectors: current_selectors, block: current_declarations, + media_types: current_media_queries + } if options[:capture_offsets] - add_rule_with_offsets!(current_selectors, current_declarations, options[:filename], (rule_start..offset.last), current_media_queries) - else - add_rule!(current_selectors, current_declarations, current_media_queries) + add_rule_options.merge!(filename: options[:filename], offset: rule_start..offset.last) end + add_rule!(**add_rule_options) end current_selectors = String.new @@ -455,11 +466,14 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc: # check for unclosed braces return unless in_declarations > 0 - unless options[:capture_offsets] - return add_rule!(current_selectors, current_declarations, current_media_queries) + add_rule_options = { + selectors: current_selectors, block: current_declarations, + media_types: current_media_queries + } + if options[:capture_offsets] + add_rule_options.merge!(filename: options[:filename], offset: rule_start..offset.last) end - - add_rule_with_offsets!(current_selectors, current_declarations, options[:filename], (rule_start..offset.last), current_media_queries) + add_rule!(**add_rule_options) end # Load a remote CSS file. diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index d423b97..7f87816 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -269,7 +269,7 @@ def initialize(*args, selectors: nil, block: nil, offset: nil, filename: nil, sp @specificity = specificity unless offset.nil? == filename.nil? - raise ArgumentError, 'require both offset and filename' + raise ArgumentError, 'require both offset and filename or no offset and no filename' end @offset = offset diff --git a/test/test_css_parser_basic.rb b/test/test_css_parser_basic.rb index 1ca8141..5efcf83 100644 --- a/test/test_css_parser_basic.rb +++ b/test/test_css_parser_basic.rb @@ -35,7 +35,7 @@ def test_adding_block_without_closing_brace end def test_adding_a_rule - @cp.add_rule!('div', 'color: blue;') + @cp.add_rule!(selectors: 'div', block: 'color: blue;') assert_equal 'color: blue;', @cp.find_by_selector('div').join(' ') end diff --git a/test/test_css_parser_media_types.rb b/test/test_css_parser_media_types.rb index 054d1b1..87e00e9 100644 --- a/test/test_css_parser_media_types.rb +++ b/test/test_css_parser_media_types.rb @@ -135,24 +135,24 @@ def test_adding_block_and_limiting_media_types end def test_adding_rule_set_with_media_type - @cp.add_rule!('body', 'color: black;', [:handheld, :tty]) - @cp.add_rule!('body', 'color: blue;', :screen) + @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: [:handheld, :tty]) + @cp.add_rule!(selectors: 'body', block: 'color: blue;', media_types: :screen) assert_equal 'color: black;', @cp.find_by_selector('body', :handheld).join(' ') end def test_adding_rule_set_with_media_query - @cp.add_rule!('body', 'color: black;', 'aural and (device-aspect-ratio: 16/9)') + @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: 'aural and (device-aspect-ratio: 16/9)') assert_equal 'color: black;', @cp.find_by_selector('body', 'aural and (device-aspect-ratio: 16/9)').join(' ') assert_equal 'color: black;', @cp.find_by_selector('body', :all).join(' ') end def test_selecting_with_all_media_types - @cp.add_rule!('body', 'color: black;', [:handheld, :tty]) + @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: [:handheld, :tty]) assert_equal 'color: black;', @cp.find_by_selector('body', :all).join(' ') end def test_to_s_includes_media_queries - @cp.add_rule!('body', 'color: black;', 'aural and (device-aspect-ratio: 16/9)') + @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: 'aural and (device-aspect-ratio: 16/9)') assert_equal "@media aural and (device-aspect-ratio: 16/9) {\n body {\n color: black;\n }\n}\n", @cp.to_s end end diff --git a/test/test_css_parser_misc.rb b/test/test_css_parser_misc.rb index bc7d17d..007a742 100644 --- a/test/test_css_parser_misc.rb +++ b/test/test_css_parser_misc.rb @@ -229,25 +229,45 @@ def test_enumerator_nonempty def test_catching_argument_exceptions_for_add_rule cp_with_exceptions = Parser.new(rule_set_exceptions: true) - assert_raises ArgumentError, 'background-color value is empty' do - cp_with_exceptions.add_rule!('body', 'background-color: !important') + cp_with_exceptions.add_rule!(selectors: 'body', block: 'background-color: !important') end cp_without_exceptions = Parser.new(rule_set_exceptions: false) + cp_without_exceptions.add_rule!(selectors: 'body', block: 'background-color: !important') + end + + def test_catching_argument_exceptions_for_add_rule_positional + cp_with_exceptions = Parser.new(rule_set_exceptions: true) - cp_without_exceptions.add_rule!('body', 'background-color: !important') + assert_raises ArgumentError, 'background-color value is empty' do + _, err = capture_io do + cp_with_exceptions.add_rule!('body', 'background-color: !important') + end + assert_includes err, "DEPRECATION" + end + + cp_without_exceptions = Parser.new(rule_set_exceptions: false) + _, err = capture_io do + cp_without_exceptions.add_rule!('body', 'background-color: !important') + end + assert_includes err, "DEPRECATION" end def test_catching_argument_exceptions_for_add_rule_with_offsets cp_with_exceptions = Parser.new(capture_offsets: true, rule_set_exceptions: true) assert_raises ArgumentError, 'background-color value is empty' do - cp_with_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1) + _, err = capture_io do + cp_with_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1) + end + assert_includes err, "DEPRECATION" end cp_without_exceptions = Parser.new(capture_offsets: true, rule_set_exceptions: false) - - cp_without_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1) + _, err = capture_io do + cp_without_exceptions.add_rule_with_offsets!('body', 'background-color: !important', 'inline', 1) + end + assert_includes err, "DEPRECATION" end end diff --git a/test/test_rule_set_creating_shorthand.rb b/test/test_rule_set_creating_shorthand.rb index 0fce89c..63262d0 100644 --- a/test/test_rule_set_creating_shorthand.rb +++ b/test/test_rule_set_creating_shorthand.rb @@ -111,7 +111,7 @@ def test_combining_dimensions_into_shorthand # Dimensions shorthand, auto property def test_combining_dimensions_into_shorthand_with_auto - rs = RuleSet.new('#page', "margin: 0; margin-left: auto; margin-right: auto;") + rs = RuleSet.new(selectors: '#page', block: "margin: 0; margin-left: auto; margin-right: auto;") rs.expand_shorthand! assert_equal('auto;', rs['margin-left']) rs.create_shorthand! @@ -201,7 +201,11 @@ def test_combining_list_style_into_shorthand end def test_property_values_in_url - rs = RuleSet.new('#header', "background:url(http://example.com/1528/www/top-logo.jpg) no-repeat top right; padding: 79px 0 10px 0; text-align:left;") + rs = RuleSet.new( + selectors: '#header', + block: "background:url(http://example.com/1528/www/top-logo.jpg) no-repeat top right; " \ + "padding: 79px 0 10px 0; text-align:left;" + ) rs.expand_shorthand! assert_equal('top right;', rs['background-position']) rs.create_shorthand! From 219d43ef282cce0dba06e558b78cf04d7e102469 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 20 May 2024 10:37:06 +0200 Subject: [PATCH 4/4] Upgrade ruby version for rubocop CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index de1975b..be682de 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,7 +35,7 @@ jobs: - uses: ruby/setup-ruby@v1 with: - ruby-version: '2.7' + ruby-version: '3.0' bundler-cache: true - run: bundle exec rake rubocop