Add Eyeglass support.

[sobolevn]

Is it a good idea to support Eyeglass?

[danielguillan]

Sure! I haven't tried Eyeglass yet, but once I have time to play with it I'll add support. A pull request for this would be very welcome too!

[sobolevn]

Ok. I have made several changes to the project, but I am not submiting a pull-request by now. There are several things to add (like npm index) that is required. Also, I have a few questions considering code style and other things, which are not covered in readme or contributing. We can continue our discution in private, just email me: mail@sobolevn.me. Also, check out changes here.

Small change log:

• Version updated to 3.0.6 (bower.json, package.json, changelog.md)
• package.json and .npmignore files created
• eyeglass-exports.js file added
• .gitattributes file added and .gitignore file updated
• mocha test added.

[danielguillan]

Thanks @sobolevn!

I think we can continue the conversation on this thread. It could benefit many people and also those who are familiar with Eyeglass can contribute with their knowledge.

Please, feel free to ask any questions you may have.

I have a few questions too:

1. Why do we need the .gitattributesfile? Isn't this a preference that you may want to store in your global git configuration?
2. I don't like to have a huge .gitignore file. I want to have things specific to the project like the sass-cache folder, the node_modules folder, … but not things that we ignore all the time like OS and IDE specific files.
3. I'm planning (Rewrite tests using True + test runner #21) to move my tests to True with Mocha and get rid of the Ruby/Compass dependency. I see that you're using Mocha for the Eyeglass tests. Can we easily combine both tests in the future?

[sobolevn]

1. My opinion is that "Explicit is better than implicit" on this one. But surely this file can be easily removed from the project.
2. The same here, I can refactor both files for only important files (assuming everything else is ignored on the global-level).
3. Yes, I think so. But I haven't really tried True tests.

Should I remove .gitattributes and refactor both .ignore files?

My question was:

1. Do you have any jshint-like style guide for Javascript files? I guess you will need one, if you are migrating to True + Mocha.
2. Does everything else look good to you?

TODO list:

1. Edit .travis.yml to run both tests. (I don't know Ruby, I am not sure that I will configure it properly). Something like that.
2. Register and upload npm package.

[danielguillan]

Should I remove .gitattributes and refactor both .ignore files?

Yes please. I totally see your point but I still prefer to only add project-related files and folders to .gitignore.

Regarding your questions:

1. Feel free to use JSHint as the style guide.
2. Everything else looks good to me.

We should also update the README file with instructions on how to install the Modernizr mixin with Eyeglass.

I'll take care of registering and pushing the package to npm once I merge your pull request.

Thanks!

[danielguillan]

@sobolevn can you confirm you're getting the correct output? I see incorrect output when compiling from both, Eyeglass and Libsass standalone. It might be related to this Libsass issue: sass/libsass#1210. The weird thing is that I did not encounter this error when I first updated the mixin to support Libsass at the time 3.2 was released.

[sobolevn]

I guess you had different patch version. The issue was submited only 20 days ago.
npm's package.json is missing dependencies and devDependencies. I was not able to run tests there. I haven't met this error before for some reason. What have I done:

1. I have tryied SassMeister on the libsass#1210 issue. The output was incorect. Here is the gist.
2. I have built new project named test with the following setup:

npm init
... [prompts] ...
npm install --save-dev node-sass eyeglass modernizr-mixin
mkdir test # and manually copied eyeglass tests there
npm test

The first test worked, so I have added 2 extra tests with this issue. There are the same as in the gist.
One passed, two failed. Full stack:

  Eyeglass module testing
    ✓ should compile a sass file (179ms)
    1) sould not create an extra parent selector
    2) sould not create an extra parent selector with modernizr-mixin


  1 passing (292ms)
  2 failing

  1) Eyeglass module testing sould not create an extra parent selector:

      Uncaught AssertionError: '.foo .foo *, .foo .foo *:before, .foo .foo *:after {\n  box-sizing: inherit;\n}\n' == '.foo *, .foo *:before, .foo *:after {\n  box-sizing: inherit;\n}\n'
      + expected - actual

      -.foo .foo *, .foo .foo *:before, .foo .foo *:after {
      +.foo *, .foo *:before, .foo *:after {
         box-sizing: inherit;
       }

      at Object.callback (test/test_eyeglass.js:35:14)

  2) Eyeglass module testing sould not create an extra parent selector with modernizr-mixin:

      Uncaught AssertionError: '.bar .js.svg .bar {\n  widht: 10px;\n}\n\n.bar .no-js .bar, .bar .no-svg .bar {\n  display: none;\n}\n' == '.js.svg .bar {\n  widht: 10px;\n}\n.no-js .bar, .no-svg .bar {\n  display: none;\n}\n'
      + expected - actual

      -.bar .js.svg .bar {
      +.js.svg .bar {
         widht: 10px;
       }
      -
      -.bar .no-js .bar, .bar .no-svg .bar {
      +.no-js .bar, .no-svg .bar {
         display: none;
       }

      at Object.callback (test/test_eyeglass.js:35:14)



npm ERR! Test failed.  See above for more details.

I can submit my new failing tests, if you want to.

[danielguillan]

I've tested with different versions of Libsass. Everything seems to work with versions 3.2.0 through 3.2.2.
It looks like a bug was introduced in Libsass 3.2.3 or 3.2.4. I've just commented on the issue: sass/libsass#1210 (comment)

[danielguillan]

I'm closing this issue and opening a new one (#24) for tracking the problem as it is not related to the Eyeglass implementation.

Thank you very much for the great work!