Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Arbitrary fixes and nested block parsing. #8

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jgm
Copy link
Contributor

@jgm jgm commented Jun 28, 2015

This fixes PR #7 and #6.

The fix to runtests.hs adds the @ for media queries, and adds a resize to limit the size of the structures, which can now be deeply nested. I also added more media query types.

The fix to Parse.hs allows indefinitely nested blocks to be parsed, and fixes the problems noted in #6.

jgm added 2 commits June 27, 2015 23:00
I left out the `@` on media queries.
This also adds a "resize" to limit the size of the structures,
which can now be nested beyond two levels.
This allows the tests amended by yesodweb#7 and yesodweb#8 to pass again.

* Simplified parsing of nested blocks.
* Indefinite nesting levels are now allowed.
* Some illegal CSS that was allowed by the old parser is now
  excluded (e.g. `@print { background: red; }`).
@jgm jgm changed the title Fixed the fixes to Arbitrary. Fixed Arbitrary fixes and nested block parsing. Jun 28, 2015
@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

It's still not right -- do not merge yet.

@jgm
Copy link
Contributor Author

jgm commented Jun 28, 2015

The more I find out about CSS, the more I think this module needs to be rewritten.
Here's a valid CSS snippet:

@page { 
  size: 21.0cm 29.7cm; /* A4 */
  @top { 
    text-align: right;
    vertical-align: center;
    content: string (chapter);
  }
}

Currently the parser gets this completely wrong:

Right [NestedBlock "@page" [LeafBlock ("size:21cm; @top",[("text-align","right")])]]

The basic data structure NestedBlock isn't quite right for this kind of thing.
In general nested blocks don't contain sequences of "regular blocks" and other nested blocks.
Rather, they contain sequences of properties and other nested blocks.

A further issue; unless we hard-code things like @font-face, there will be an ambiguity with @font-face{} -- is it a NestedBlock or a LeafBlock? Though this would also be solved if we made the change I just suggested. Get rid of NestedBlock and CSSBlock, and just have something like

data Element = Stanza Selector [Element] | Property
type Property = (Text, Text)
type Selector = Text

Looking carefully at the CSS syntax definition might yield better ideas.

Testing with GHC 7.10 has its drawbacks!
@srghma srghma mentioned this pull request Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant