Space in font names lost #6

Closed
Piedone opened this Issue Sep 20, 2012 · 10 comments

Projects

None yet

2 participants

Piedone commented Sep 20, 2012

A CSS like this:

body
{
font: normal 100% Frutiger, "Frutiger Linotype" , Univers, Calibri, "Gill Sans" , "Gill Sans MT" , "Myriad Pro" , Myriad, "DejaVu Sans Condensed" , "Liberation Sans" , "Nimbus Sans L" , Geneva, "Helvetica Neue" , Helvetica, Arial, sans-serif;
}

after parsed will have font names without spaces, like FrutigerLinotype.
What can I do? Thanks in advance!

Owner

I'm perfectly happy to take a pull request.

Date: Thu, 20 Sep 2012 14:14:29 -0700
From: notifications@github.com
To: ExCSS@noreply.github.com
Subject: [ExCSS] Space in font names lost (#6)

A CSS like this:

body

{

font: normal 100% Frutiger, "Frutiger Linotype" , Univers, Calibri, "Gill Sans" , "Gill Sans MT" , "Myriad Pro" , Myriad, "DejaVu Sans Condensed" , "Liberation Sans" , "Nimbus Sans L" , Geneva, "Helvetica Neue" , Helvetica, Arial, sans-serif;

}

after parsed will have font names without spaces, like FrutigerLinotype.

What can I do?

          —

          Reply to this email directly or view it on GitHub.
Piedone commented Sep 20, 2012

Well, if you pinpoint me where to look for the issue I can make a try :-).

Owner

It's hard to know just where to point you. Sadly, Coco/r is not white space sensitive. There are a few functions at the top of the ATG file that try to help where Coco/r fails. Probably the best place to look would be the unit tests. You'll see tests for most areas of the code base. I'd add one that fails and then see on or about what area of the parser causes the trouble. I've done what I can to document the ATG file so it can be tracked down.

I hope that's clear enough. I'll try the doing same and share my findings. Another pair of eyes is always better!

Date: Thu, 20 Sep 2012 14:40:56 -0700
From: notifications@github.com
To: ExCSS@noreply.github.com
CC: tylerbrinks@hotmail.com
Subject: Re: [ExCSS] Space in font names lost (#6)

Well, if you pinpoint me where to look for the issue I can make a try :-).

          —

          Reply to this email directly or view it on GitHub.
Piedone commented Sep 26, 2012

Oh damn, I forgot the parser was generated from Coco/r... The problem is that this is completely foreign to me. Nevertheless I'll look at the places you advised.

Owner

Yea - not exactly an easy "language" to learn. Attributed Grammar is... interesting. My hope in the long run is to either switch to a more readable and white space friendly parser like Ragel, or try to port Chrome's CSS Engine. Chrome uses a parses as well, but for their lower level C/C++ code.

Date: Wed, 26 Sep 2012 14:11:12 -0700
From: notifications@github.com
To: ExCSS@noreply.github.com
CC: tylerbrinks@hotmail.com
Subject: Re: [ExCSS] Space in font names lost (#6)

Oh damn, I forgot the parser was generated from Coco/r... The problem is that this is completely foreign to me. Nevertheless I'll look at the places you advised.

          —

          Reply to this email directly or view it on GitHub.
Piedone commented Sep 29, 2012

I forked the project and after managing to build it (due to the path containing spaces the copy command initially failed, thus building too) I can't run the tests with the TestDriven.Net plugin (it starts, then immediately finishes and says that run = failed = succeeded = 0 tests...). Is there some special config needed?

Piedone commented Sep 29, 2012

Now I started experimenting around by simply running the project with a test console app. It turns out that if the font names are not in quotes the spaces remain. Since the quoted version is also valid CSS, the problem remains, however.
I digged around a bit with the debugger, here are my findings:
Probably root of the problem is that a space-separated, quote font name is recognized as two tokens. Its parsing goes through Parser.QuotedStringPreserved() and since there tokens are concatenated on ln 403 the space goes away. The root of this is that in Scanner.NextToken() ln 397 if the current character is a space the expression returns false, so on ln 398 the value of the variable t will be a token only containing the first part of the font name.
What do you think?

Piedone commented Oct 9, 2012

Hi, any updates on this?

@Piedone Piedone closed this Oct 9, 2012
@Piedone Piedone reopened this Oct 9, 2012
Owner

I have a bit of update - more of a change in direction. I've reached the end of what Coco/r is capable of. It's simply not the right tool for the job given it's inability to parse white space properly. I'm working on converting the whole project over to use Ragel as the lexer generator.

Piedone commented Oct 16, 2012

Oh wow. Thanks for the update, this looks great.
Any estimate on when you can complete the converting?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment