From 8bd287cf78a1df55888bd20d57495eff277e67e9 Mon Sep 17 00:00:00 2001 From: Matthew DuVall Date: Thu, 11 Apr 2013 22:32:26 -0700 Subject: [PATCH 1/3] 0 and 0px lengths treated equally as parser rules --- lib/csscss/parser/common.rb | 24 +++++++++++++----------- test/csscss/parser/common_test.rb | 1 + 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/csscss/parser/common.rb b/lib/csscss/parser/common.rb index 4a3d965..20a1fce 100644 --- a/lib/csscss/parser/common.rb +++ b/lib/csscss/parser/common.rb @@ -5,17 +5,19 @@ module Common UNITS = %w(px em ex in cm mm pt pc) - rule(:space) { match['\s'].repeat(1) } - rule(:space?) { space.maybe } - rule(:number) { match["0-9"] } - rule(:numbers) { number.repeat(1) } - rule(:decimal) { numbers >> str(".").maybe >> numbers.maybe } - rule(:percent) { decimal >> stri("%") >> space? } - rule(:length) { decimal >> stri_list(UNITS) >> space? } - rule(:identifier) { match["a-zA-Z"].repeat(1) } - rule(:inherit) { stri("inherit") } - rule(:eof) { any.absent? } - rule(:nada) { any.repeat.as(:nada) } + rule(:space) { match['\s'].repeat(1) } + rule(:space?) { space.maybe } + rule(:number) { match["0-9"] } + rule(:numbers) { number.repeat(1) } + rule(:decimal) { numbers >> str(".").maybe >> numbers.maybe } + rule(:percent) { decimal >> stri("%") >> space? } + rule(:non_zero_length) { decimal >> stri_list(UNITS) >> space? } + rule(:zero_length) { match["0"] } + rule(:length) { non_zero_length | zero_length } + rule(:identifier) { match["a-zA-Z"].repeat(1) } + rule(:inherit) { stri("inherit") } + rule(:eof) { any.absent? } + rule(:nada) { any.repeat.as(:nada) } rule(:http) { (match['a-zA-Z0-9.:/\-'] | str('\(') | str('\)')).repeat >> space? diff --git a/test/csscss/parser/common_test.rb b/test/csscss/parser/common_test.rb index 75fbde6..06486b8 100644 --- a/test/csscss/parser/common_test.rb +++ b/test/csscss/parser/common_test.rb @@ -106,6 +106,7 @@ class CommonTest @parser.length.must_parse "123px" @parser.length.must_parse "123EM" @parser.length.must_parse "1.23Pt" + @parser.length.must_parse "0" end end From 936460c2a5d60c7176744c6823460788fa06cc97 Mon Sep 17 00:00:00 2001 From: Matthew DuVall Date: Thu, 11 Apr 2013 23:28:28 -0700 Subject: [PATCH 2/3] Add to parse to normalize 0 length values --- lib/csscss/parser/common.rb | 2 +- lib/csscss/redundancy_analyzer.rb | 1 - lib/csscss/types.rb | 14 +++++++++++++- test/csscss/redundancy_analyzer_test.rb | 12 ++++++++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/csscss/parser/common.rb b/lib/csscss/parser/common.rb index 20a1fce..2885b89 100644 --- a/lib/csscss/parser/common.rb +++ b/lib/csscss/parser/common.rb @@ -13,7 +13,7 @@ module Common rule(:percent) { decimal >> stri("%") >> space? } rule(:non_zero_length) { decimal >> stri_list(UNITS) >> space? } rule(:zero_length) { match["0"] } - rule(:length) { non_zero_length | zero_length } + rule(:length) { zero_length | non_zero_length } rule(:identifier) { match["a-zA-Z"].repeat(1) } rule(:inherit) { stri("inherit") } rule(:eof) { any.absent? } diff --git a/lib/csscss/redundancy_analyzer.rb b/lib/csscss/redundancy_analyzer.rb index 04a1109..3f1882c 100644 --- a/lib/csscss/redundancy_analyzer.rb +++ b/lib/csscss/redundancy_analyzer.rb @@ -64,7 +64,6 @@ def redundancies(opts = {}) end end - # trims any derivative declarations alongside shorthand inverted_matches.each do |selectors, declarations| redundant_derivatives = declarations.select do |dec| diff --git a/lib/csscss/types.rb b/lib/csscss/types.rb index 0cbcd74..1ca5ccd 100644 --- a/lib/csscss/types.rb +++ b/lib/csscss/types.rb @@ -5,7 +5,7 @@ def self.from_csspool(dec) end def self.from_parser(property, value, clean = true) - value = value.to_s + value = normalize_value(value) property = property.to_s if clean value = value.downcase @@ -14,6 +14,18 @@ def self.from_parser(property, value, clean = true) new(property, value.strip) end + def self.normalize_value(value) + value = value.to_s.strip + return value if value.nil? + + zero_regex_matches = value.match(/^(0)(.*)/) + if zero_regex_matches && zero_regex_matches.length + zero_regex_matches[1] # "0" + else + value + end + end + def derivative? !parents.nil? end diff --git a/test/csscss/redundancy_analyzer_test.rb b/test/csscss/redundancy_analyzer_test.rb index d37283a..2f74e95 100644 --- a/test/csscss/redundancy_analyzer_test.rb +++ b/test/csscss/redundancy_analyzer_test.rb @@ -266,6 +266,18 @@ module Csscss }) end + it "matches 0 and 0px" do + css = %$ + .bar { padding: 0; } + .foo { padding: 0px; } + $ + + RedundancyAnalyzer.new(css).redundancies.must_equal({ + [sel(".bar"), sel(".foo")] => [dec("padding", "0")] + }) + end + + # TODO: someday # it "reports duplication within the same selector" do # css = %$ From 3f361f56e8bb6aabd4cf682b10c8fafdbcc47508 Mon Sep 17 00:00:00 2001 From: Matthew DuVall Date: Fri, 12 Apr 2013 16:07:12 -0700 Subject: [PATCH 3/3] Move normalization into hash function, Update CHANGELOG, CONTRIBUTORS --- CHANGELOG.md | 1 + CONTRIBUTORS.md | 1 + lib/csscss/types.rb | 28 ++++++++++++++-------------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0abc57..68dc4f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Adds --compass-with-config that lets users specify a config * Fixes parser error bug when trying to parse blank files * Fixes bug where rules from multiple files aren't consolidated +* Fixes bug where 0 and 0px are not reconciled as redundancies ## 1.0.0 - 4/7/2013 ## diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index cd60c0a..cc2d250 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -1,3 +1,4 @@ * Zach Moazeni @zmoazeni * Carson McDonald @carsonmcdonald * Martin Kuckert @MKuckert +* Matt DuVall @mduvall_ diff --git a/lib/csscss/types.rb b/lib/csscss/types.rb index 1ca5ccd..26d8155 100644 --- a/lib/csscss/types.rb +++ b/lib/csscss/types.rb @@ -5,7 +5,7 @@ def self.from_csspool(dec) end def self.from_parser(property, value, clean = true) - value = normalize_value(value) + value = value.to_s property = property.to_s if clean value = value.downcase @@ -14,18 +14,6 @@ def self.from_parser(property, value, clean = true) new(property, value.strip) end - def self.normalize_value(value) - value = value.to_s.strip - return value if value.nil? - - zero_regex_matches = value.match(/^(0)(.*)/) - if zero_regex_matches && zero_regex_matches.length - zero_regex_matches[1] # "0" - else - value - end - end - def derivative? !parents.nil? end @@ -48,8 +36,20 @@ def ==(other) end end + def normalize_value(value) + value = value.to_s.strip + return value if value.nil? + + zero_regex_matches = value.match(/^(0)(.*)/) + if zero_regex_matches && zero_regex_matches.length + zero_regex_matches[1] # "0" + else + value + end + end + def hash - [property, value].hash + [property, normalize_value(value)].hash end def eql?(other)