V2 rule set declaration parsing #159
Conversation
stoivo
force-pushed
the
v2-rule-set-declaration-parsing
branch
from
cb93ddc
to
06acf7a
Compare
lib/css_parser/parser.rb
Outdated
| # We have a Parser class which you create and instance of but we have some | ||
| # functions which is nice to have outside of this instance | ||
| module ParserFx |
There was a problem hiding this comment.
thoughts on using self. methods for this ?
(just style so nbd, but it's what I would have expected)
There was a problem hiding this comment.
I was thinking I could do that but I think the Parser class does so much already, and that class is public API so if I add a class method to Parser it public and open for use but I don't want that.
Also I am thinking we should maby change the public API to something like CssParser::Document.new, And on this instance we have method like add_block, load_url, each_rule, find_by_selector, etc.
There was a problem hiding this comment.
could make the methods private with private_class_method or class << self and private
ParserFx is also not private so kinda similar
if they are unrelated methods it's nice to get them out of the parser, but not sure they really are
ideally I'd want to keep the general structure the same so the upgrade instructions are just "replace method call a with b and change the args", but if you see a good reason then 👍 since it's v2 anyways
There was a problem hiding this comment.
I also want to keep the structure the same so it's just upgrade and your good.
I need a way to share it with RuleSet instance so I don't think private class method is gooding to work
| @@ -203,6 +203,85 @@ def expand_dimensions_shorthand! # :nodoc: | |||
| end | |||
| end | |||
|
|
|||
| class FontScanner | |||
stoivo
force-pushed
the
v2-rule-set-declaration-parsing
branch
from
06acf7a
to
38bb6ea
Compare
|
I need to investigate what happening here, never worked with Jruby before |
stoivo
force-pushed
the
v2-rule-set-declaration-parsing
branch
2 times, most recently
from
2b26ea3
to
fdf5cb2
Compare
|
I don't know whats the matter with Jruby. Now it works with no change. Could be network issue Not you are running |
stoivo
force-pushed
the
v2-rule-set-declaration-parsing
branch
2 times, most recently
from
38bb6ea
to
57467b9
Compare
I think this is a feature we can remove in v2. The consumer should strip
{} if they have them.
Added 13 years ago without much for a conversation
premailer#13
firefox will not evaluate declaration prop `font: 300 italic auto/14px verdana, helvetica, sans-serif;` becase auto isn't valid
I used https://www.w3.org/TR/CSS21/fonts.html#propdef-font to create the scanner
stoivo
force-pushed
the
v2-rule-set-declaration-parsing
branch
from
57467b9
to
2ce3d67
Compare
|
I think we can merge this |
| @@ -14,6 +14,7 @@ Gem::Specification.new name, CssParser::VERSION do |s| | |||
| s.required_ruby_version = '>= 3.0' | |||
|
|
|||
| s.metadata['changelog_uri'] = 'https://github.com/premailer/css_parser/blob/master/CHANGELOG.md' | |||
| s.metadata['documentation_uri'] = "https://www.rubydoc.info/gems/css_parser/#{s.version}" | |||
There was a problem hiding this comment.
idk if this is still getting updated might be better to point to github
I refactored expand_font_shorthand. I think its better. I think I want to do the same for all the other expand features. When I do that I think we can remove more of the regexes. Would you like that?
I am trying not to touch test too much to keep all existing features working.
Should I maybe add some notes into changelog that some features are removed?