From 9145f6e98d08452e1fe1f317b97564cb6d3730b3 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 31 Jan 2021 21:14:21 -0800 Subject: [PATCH 1/2] kiss style --- .rubocop.yml | 5 ++++- Rakefile | 4 ++-- lib/css_parser/rule_set.rb | 10 +++++----- test/test_css_parser_media_types.rb | 6 +++--- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 0f3ce72..5df8a41 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,5 @@ AllCops: - TargetRubyVersion: 2.4 + TargetRubyVersion: 2.4 # lowest supported version NewCops: enable Layout/ArgumentAlignment: @@ -68,3 +68,6 @@ Style/StringLiterals: Style/WordArray: Enabled: false + +Style/SymbolArray: + EnforcedStyle: brackets diff --git a/Rakefile b/Rakefile index 35522bd..021cd23 100644 --- a/Rakefile +++ b/Rakefile @@ -6,6 +6,8 @@ require 'rake/testtask' require 'rubocop/rake_task' require 'bump/tasks' +task default: [:rubocop, :test] + Rake::TestTask.new do |test| test.pattern = 'test/**/test*.rb' test.verbose = true @@ -37,5 +39,3 @@ task :benchmark do report = MemoryProfiler.report { CssParser::Parser.new.load_file!(complex_css_path) } puts "Loading `complex.css` allocated #{report.total_allocated} objects, #{report.total_allocated_memsize / 1024} KiB" end - -task default: %i[rubocop test] diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index 7e97394..d744e2d 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -529,7 +529,7 @@ def create_dimensions_shorthand! # :nodoc: return if declarations.size < NUMBER_OF_DIMENSIONS DIMENSIONS.each do |property, dimensions| - values = %i[top right bottom left].each_with_index.with_object({}) do |(side, index), result| + values = [:top, :right, :bottom, :left].each_with_index.with_object({}) do |(side, index), result| next unless declarations.key?(dimensions[index]) result[side] = declarations[dimensions[index]].value @@ -586,15 +586,15 @@ def create_list_style_shorthand! # :nodoc: def compute_dimensions_shorthand(values) # All four sides are equal, returning single value - return %i[top] if values.values.uniq.count == 1 + return [:top] if values.values.uniq.count == 1 # `/* top | right | bottom | left */` - return %i[top right bottom left] if values[:left] != values[:right] + return [:top, :right, :bottom, :left] if values[:left] != values[:right] # Vertical are the same & horizontal are the same, `/* vertical | horizontal */` - return %i[top left] if values[:top] == values[:bottom] + return [:top, :left] if values[:top] == values[:bottom] - %i[top left bottom] + [:top, :left, :bottom] end def parse_declarations!(block) # :nodoc: diff --git a/test/test_css_parser_media_types.rb b/test/test_css_parser_media_types.rb index 11a0f54..6f3ff27 100644 --- a/test/test_css_parser_media_types.rb +++ b/test/test_css_parser_media_types.rb @@ -59,7 +59,7 @@ def test_finding_by_multiple_media_types } CSS - assert_equal 'font-size: 13px; line-height: 1.2;', @cp.find_by_selector('body', %i[screen handheld]).join(' ') + assert_equal 'font-size: 13px; line-height: 1.2;', @cp.find_by_selector('body', [:screen, :handheld]).join(' ') end def test_adding_block_with_media_types @@ -116,7 +116,7 @@ def test_adding_block_and_limiting_media_types end def test_adding_rule_set_with_media_type - @cp.add_rule!('body', 'color: black;', %i[handheld tty]) + @cp.add_rule!('body', 'color: black;', [:handheld, :tty]) @cp.add_rule!('body', 'color: blue;', :screen) assert_equal 'color: black;', @cp.find_by_selector('body', :handheld).join(' ') end @@ -128,7 +128,7 @@ def test_adding_rule_set_with_media_query end def test_selecting_with_all_media_types - @cp.add_rule!('body', 'color: black;', %i[handheld tty]) + @cp.add_rule!('body', 'color: black;', [:handheld, :tty]) assert_equal 'color: black;', @cp.find_by_selector('body', :all).join(' ') end From 8868aa69eb4369161a89765df11ab2537915c968 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Sun, 31 Jan 2021 22:12:05 -0800 Subject: [PATCH 2/2] cleanup --- lib/css_parser/regexps.rb | 2 + lib/css_parser/rule_set.rb | 183 +++++++++++++++++++------------------ 2 files changed, 97 insertions(+), 88 deletions(-) diff --git a/lib/css_parser/regexps.rb b/lib/css_parser/regexps.rb index 0aa9a44..9b3bcf6 100644 --- a/lib/css_parser/regexps.rb +++ b/lib/css_parser/regexps.rb @@ -22,6 +22,7 @@ def self.regex_possible_values(*values) RE_URI = /(url\(\s*(\s*#{RE_STRING}\s*)\s*\))|(url\(\s*([!#$%&*\-~]|#{RE_NON_ASCII}|#{RE_ESCAPE})*\s*)\)/ixm.freeze URI_RX = /url\(("([^"]*)"|'([^']*)'|([^)]*))\)/im.freeze + URI_RX_OR_NONE = Regexp.union(URI_RX, /none/i) RE_GRADIENT = /[-a-z]*gradient\([-a-z0-9 .,#%()]*\)/im.freeze # Initial parsing @@ -43,6 +44,7 @@ def self.regex_possible_values(*values) 'upper-latin', 'hebrew', 'armenian', 'georgian', 'cjk-ideographic', 'hiragana', 'hira-gana-iroha', 'katakana-iroha', 'katakana', 'none' ) + RE_IMAGE = Regexp.union(CssParser::URI_RX, CssParser::RE_GRADIENT, /none/i) STRIP_CSS_COMMENTS_RX = %r{/\*.*?\*/}m.freeze STRIP_HTML_COMMENTS_RX = //m.freeze diff --git a/lib/css_parser/rule_set.rb b/lib/css_parser/rule_set.rb index d744e2d..793e80f 100644 --- a/lib/css_parser/rule_set.rb +++ b/lib/css_parser/rule_set.rb @@ -44,9 +44,7 @@ def value=(value) end def to_s - return value unless important - - "#{value} !important" + important ? "#{value} !important" : value end def ==(other) @@ -88,15 +86,11 @@ def []=(property, value) if value.is_a?(Value) declarations[property] = value - return - end - - if value.to_s.strip.empty? - delete(property) - return + elsif value.to_s.strip.empty? + delete property + else + declarations[property] = Value.new(value) end - - declarations[property] = Value.new(value) end alias add_declaration! []= @@ -153,14 +147,18 @@ def replace_declaration!(property, replacements, preserve_importance: false) # We should preserve subsequent declarations of the same properties # and prior important ones if replacement one is not important - replacements = replacement_declarations.each.with_object({}) do |(key, value), result| - # Replacement property doesn't exist, adding - next result[key] = value unless declarations.key?(key) - - # Replacement property is important while existing one is not, - # replacing unconditionally - if value.important && !declarations[key].important - result[key] = value + replacements = replacement_declarations.each.with_object({}) do |(key, replacement), result| + existing = declarations[key] + + # No existing -> set + unless existing + result[key] = replacement + next + end + + # Replacement more important than existing -> replace + if replacement.important && !existing.important + result[key] = replacement replaced_index = replacement_keys.index(key) replacement_keys.delete_at(replaced_index) replacement_values.delete_at(replaced_index) @@ -168,13 +166,12 @@ def replace_declaration!(property, replacements, preserve_importance: false) next end - # Existing value is important while replacing is not, existing one - # takes precedence - next if !value.important && declarations[key].important + # Existing is more important than replacement -> keep + next if !replacement.important && existing.important - # Importance of existing and replacement values are the same, + # Existing and replacement importance are the same, # value which is declared later wins - result[key] = value if property_index > replacement_keys.index(key) + result[key] = replacement if property_index > replacement_keys.index(key) end return if replacements.empty? @@ -245,9 +242,9 @@ def initialize(selectors, block, specificity = nil) # Get the value of a property def get_value(property) - return '' unless declarations.key?(property) + return '' unless (value = declarations[property]) - "#{declarations[property]};" + "#{value};" end alias [] get_value @@ -301,19 +298,23 @@ def expand_shorthand! # # See http://www.w3.org/TR/CSS21/colors.html#propdef-background def expand_background_shorthand! # :nodoc: - return unless declarations.key?('background') - - value = declarations['background'].value.dup - - replacement = BACKGROUND_PROPERTIES.map { |key| [key, 'inherit'] }.to_h if value.match(CssParser::RE_INHERIT) - replacement ||= { - 'background-image' => value.slice!(Regexp.union(CssParser::URI_RX, CssParser::RE_GRADIENT, /none/i)), - 'background-attachment' => value.slice!(CssParser::RE_SCROLL_FIXED), - 'background-repeat' => value.slice!(CssParser::RE_REPEAT), - 'background-color' => value.slice!(CssParser::RE_COLOUR), - 'background-size' => extract_background_size_from(value), - 'background-position' => value.slice!(CssParser::RE_BACKGROUND_POSITION) - } + return unless (declaration = declarations['background']) + + value = declaration.value.dup + + replacement = + if value.match(CssParser::RE_INHERIT) + BACKGROUND_PROPERTIES.map { |key| [key, 'inherit'] }.to_h + else + { + 'background-image' => value.slice!(CssParser::RE_IMAGE), + 'background-attachment' => value.slice!(CssParser::RE_SCROLL_FIXED), + 'background-repeat' => value.slice!(CssParser::RE_REPEAT), + 'background-color' => value.slice!(CssParser::RE_COLOUR), + 'background-size' => extract_background_size_from(value), + 'background-position' => value.slice!(CssParser::RE_BACKGROUND_POSITION) + } + end declarations.replace_declaration!('background', replacement, preserve_importance: true) end @@ -328,9 +329,9 @@ def extract_background_size_from(value) # Additional splitting happens in expand_dimensions_shorthand! def expand_border_shorthand! # :nodoc: BORDER_PROPERTIES.each do |k| - next unless declarations.key?(k) + next unless (declaration = declarations[k]) - value = declarations[k].value.dup + value = declaration.value.dup replacement = { "#{k}-width" => value.slice!(CssParser::RE_BORDER_UNITS), @@ -346,9 +347,9 @@ def expand_border_shorthand! # :nodoc: # into their constituent parts. Handles margin, padding, border-color, border-style and border-width. def expand_dimensions_shorthand! # :nodoc: DIMENSIONS.each do |property, (top, right, bottom, left)| - next unless declarations.key?(property) + next unless (declaration = declarations[property]) - value = declarations[property].value.dup + value = declaration.value.dup # RGB and HSL values in borders are the only units that can have spaces (within params). # We cheat a bit here by stripping spaces after commas in RGB and HSL values so that we @@ -369,38 +370,39 @@ def expand_dimensions_shorthand! # :nodoc: values << matches[1] # left = right when 4 values = matches.to_a + else + raise ArgumentError, "Cannot parse #{value}" end t, r, b, l = values + replacement = {top => t, right => r, bottom => b, left => l} - declarations.replace_declaration!( - property, - {top => t, right => r, bottom => b, left => l}, - preserve_importance: true - ) + declarations.replace_declaration!(property, replacement, preserve_importance: true) end end # Convert shorthand font declarations (e.g. font: 300 italic 11px/14px verdana, helvetica, sans-serif;) # into their constituent parts. def expand_font_shorthand! # :nodoc: - return unless declarations.key?('font') - - font_props = {} + return unless (declaration = declarations['font']) # reset properties to 'normal' per http://www.w3.org/TR/CSS21/fonts.html#font-shorthand - ['font-style', 'font-variant', 'font-weight', 'font-size', 'line-height'].each do |prop| - font_props[prop] = 'normal' - end + font_props = { + 'font-style' => 'normal', + 'font-variant' => 'normal', + 'font-weight' => 'normal', + 'font-size' => 'normal', + 'line-height' => 'normal' + } - value = declarations['font'].value.dup + value = declaration.value.dup value.gsub!(%r{/\s+}, '/') # handle spaces between font size and height shorthand (e.g. 14px/ 16px) in_fonts = false - matches = value.scan(/("(.*[^"])"|'(.*[^'])'|(\w[^ ,]+))/) - matches.each do |match| - m = match[0].to_s.strip + matches = value.scan(/"(?:.*[^"])"|'(?:.*[^'])'|(?:\w[^ ,]+)/) + matches.each do |m| + m.strip! m.gsub!(/;$/, '') if in_fonts @@ -411,7 +413,7 @@ def expand_font_shorthand! # :nodoc: end elsif m =~ /normal|inherit/i ['font-style', 'font-weight', 'font-variant'].each do |font_prop| - font_props[font_prop] = m unless font_props.key?(font_prop) + font_props[font_prop] ||= m end elsif m =~ /italic|oblique/i font_props['font-style'] = m @@ -420,8 +422,8 @@ def expand_font_shorthand! # :nodoc: elsif m =~ /[1-9]00$|bold|bolder|lighter/i font_props['font-weight'] = m elsif m =~ CssParser::FONT_UNITS_RX - if m =~ %r{/} - font_props['font-size'], font_props['line-height'] = m.split('/') + if m.include?('/') + font_props['font-size'], font_props['line-height'] = m.split('/', 2) else font_props['font-size'] = m end @@ -437,16 +439,20 @@ def expand_font_shorthand! # :nodoc: # # See http://www.w3.org/TR/CSS21/generate.html#lists def expand_list_style_shorthand! # :nodoc: - return unless declarations.key?('list-style') - - value = declarations['list-style'].value.dup - - replacement = LIST_STYLE_PROPERTIES.map { |key| [key, 'inherit'] }.to_h if value =~ CssParser::RE_INHERIT - replacement ||= { - 'list-style-type' => value.slice!(CssParser::RE_LIST_STYLE_TYPE), - 'list-style-position' => value.slice!(CssParser::RE_INSIDE_OUTSIDE), - 'list-style-image' => value.slice!(Regexp.union(CssParser::URI_RX, /none/i)) - } + return unless (declaration = declarations['list-style']) + + value = declaration.value.dup + + replacement = + if value =~ CssParser::RE_INHERIT + LIST_STYLE_PROPERTIES.map { |key| [key, 'inherit'] }.to_h + else + { + 'list-style-type' => value.slice!(CssParser::RE_LIST_STYLE_TYPE), + 'list-style-position' => value.slice!(CssParser::RE_INSIDE_OUTSIDE), + 'list-style-image' => value.slice!(CssParser::URI_RX_OR_NONE) + } + end declarations.replace_declaration!('list-style', replacement, preserve_importance: true) end @@ -466,10 +472,11 @@ def create_shorthand_properties!(properties, shorthand_property) # :nodoc: values = [] properties_to_delete = [] properties.each do |property| - if declarations.key?(property) and not declarations[property].important - values << declarations[property].value - properties_to_delete << property - end + next unless (declaration = declarations[property]) + next if declaration.important + + values << declaration.value + properties_to_delete << property end return if values.length <= 1 @@ -490,12 +497,9 @@ def create_background_shorthand! # :nodoc: # background-position by preceding it with a backslash. In this case we also need to # have a background-position property, so we set it if it's missing. # http://www.w3schools.com/cssref/css3_pr_background.asp - if declarations.key?('background-size') and not declarations['background-size'].important - unless declarations.key?('background-position') - declarations['background-position'] = '0% 0%' - end - - declarations['background-size'].value = "/ #{declarations['background-size'].value}" + if (declaration = declarations['background-size']) && !declaration.important + declarations['background-position'] ||= '0% 0%' + declaration.value = "/ #{declaration.value}" end create_shorthand_properties! BACKGROUND_PROPERTIES, 'background' @@ -509,12 +513,15 @@ def create_border_shorthand! # :nodoc: values = [] BORDER_STYLE_PROPERTIES.each do |property| - next unless declarations.key?(property) and not declarations[property].important + next unless (declaration = declarations[property]) + next if declaration.important + # can't merge if any value contains a space (i.e. has multiple values) # we temporarily remove any spaces after commas for the check (inside rgba, etc...) - return nil if declarations[property].value.gsub(/,\s/, ',').strip =~ /\s/ + return nil if declaration.value.gsub(/,\s/, ',').strip =~ /\s/ + + values << declaration.value - values << declarations[property].value declarations.delete(property) end @@ -530,9 +537,9 @@ def create_dimensions_shorthand! # :nodoc: DIMENSIONS.each do |property, dimensions| values = [:top, :right, :bottom, :left].each_with_index.with_object({}) do |(side, index), result| - next unless declarations.key?(dimensions[index]) + next unless (declaration = declarations[dimensions[index]]) - result[side] = declarations[dimensions[index]].value + result[side] = declaration.value end # All four dimensions must be present @@ -604,10 +611,10 @@ def parse_declarations!(block) # :nodoc: continuation = nil block.split(/[;$]+/m).each do |decs| - decs = continuation ? continuation + decs : decs + decs = (continuation ? continuation + decs : decs) if decs =~ /\([^)]*\Z/ # if it has an unmatched parenthesis continuation = "#{decs};" - elsif (matches = decs.match(/\s*(.[^:]*)\s*:\s*(.+?)(;?\s*\Z)/i)) + elsif (matches = decs.match(/\s*(.[^:]*)\s*:\s*(.+?)(?:;?\s*\Z)/i)) # skip end_of_declaration property = matches[1] value = matches[2]