Interpolation confuses namingConvention setting sometimes

[simenb]

Interpolation confuses namingConvention setting sometimes

[rossPatton]

Thanks for the super in-depth examples, I'll take a look at it tonight

[rossPatton]

Hey, so, i've pushed some changes up. Can you pull and play with it a bit? I think i've covered all the examples listed here but you might be able to find some edge cases I can't.

Thanks!

[awayken]

@rossPatton: It looks like master is where your latest commits are, is that right? How do you recommend I test the updates? Check out the project and run node index.js in place of stylint?

[rossPatton]

yep, that should work

[awayken]

Almost all the cases where I had issues are now working. Awesome!

However, there is one case where I still have issues:

$class-name = error
$dark-red = maroon

.block_{$class-name} {
    color: $dark-red;
}

[rossPatton]

I'll take a look at this case probably tomorrow, and hopefully get it up on npm this weekend. Thanks for testing!

[awayken]

I'm happy to help. I've been craving a tool like this ever since I started using Stylus, so I'll do whatever I can to help make it robust.

[rossPatton]

However you want to contribute is fine by me, in-depth bug reporting like this in particular is very helpful, but if you have any other ways you can contribute i'm open to collaboration.

Regarding this current bug: if you could pull down my latest, that one last case should be resolved. If you wanna double check that for me that'd be great.

[awayken]

@rossPatton: My test file is still not passing.

How do I got about running the test suites myself? I ran npm install and then npm test, but I don't have a module called "istanbul" installed. Is that something that should be listed as a devDependency? If I install npm install -g istanbul, is there anything else I need to install to run tests?

[rossPatton]

@awayken Huh, really? After pulling the latest? All my tests are passing, and that includes your last example. Maybe I botched my unit tests?

If you want, you're welcome to fork this and make changes. And yeah, istanbul needs to be installed, and should probably be listed as a dev dependency. Istanbul is what I use with mocha to generate code coverage reports. That should be all you need. Then just run npm test.

[awayken]

@rossPatton: Yeah, I pulled the latest. I do see some areas where the unit tests might not be quite right.

Like line 722:

assert.equal( false, app.namingConvention('.class_name_like_this', 'lowercase-dash') );

I feel it should be:

assert.equal( true, app.namingConvention('.class_name_like_this', 'lowercase-dash') );

Because CSS class names don't need to follow the rules of the Stylus variable naming convention. At least, that's the way I'd like to see it.

Relevant to the issue I'm still having above, I saw this line (729), which also looks like it should equal true instead of false:

assert.equal( false, app.namingConvention('.block_{$class-name}', 'lowercase-dash') );

With regards to helping test things, I installed Istanbul and ran npm test but I got some errors. I wonder if it's because I'm running iojs instead of true nodejs. Or maybe it's because I'm on Windows. What OS and platform do you develop on?

[rossPatton]

Hey, thanks for continuing to look at this. I'm at work right now, but i'll take another look tonight.

As far as my development environment goes, i'm on a mac, usually running node but i have run iojs before with no issues. Mocha/Chai/Sinon/Istanbul for testing. What errors are you getting?

[awayken]

My issue seems to point back to the _mocha call. Google has not been a ton of help so far. What I've found seems to suggest that it might be a Windows thing. (Lucky me...)

D:\Sites\stylint [master +0 ~0 -0]> npm test

> [email protected] test D:\Sites\stylint
> istanbul cover node_modules/.bin/_mocha -- -u exports -R spec test/test.js

No coverage information was collected, exit without writing coverage information
D:\Sites\stylint\node_modules\.bin\_mocha:4
case `uname` in
^^^^
SyntaxError: Unexpected token case
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:427:25)
    at Module._extensions..js (module.js:462:10)
    at Object.Module._extensions..js (C:\Users\mrausch\AppData\Roaming\npm\node_modules\istanbul\lib\hook.js:102:13)
    at Module.load (module.js:339:32)
    at Function.Module._load (module.js:294:12)
    at Function.Module.runMain (module.js:485:10)
    at runFn (C:\Users\mrausch\AppData\Roaming\npm\node_modules\istanbul\lib\command\common\run-with-cover.js:120:16)
    at C:\Users\mrausch\AppData\Roaming\npm\node_modules\istanbul\lib\command\common\run-with-cover.js:246:17
    at C:\Users\mrausch\AppData\Roaming\npm\node_modules\istanbul\lib\util\file-matcher.js:68:16
npm ERR! Test failed.  See above for more details.

[rossPatton]

@awayken have you tried just running mocha directly ?

[rossPatton]

@awayken regarding the two tests you've pulled out, I actually do think that classes and ids and such should follow the same naming convention as variables. And for cases where you don't have control over the class (ads, 3rd party tools), i'd recommend using the @stylint off to keep it from throwing warnings.

I admit this could get cumbersome if you have a lot of these kind of classes, so open to ideas there.

[awayken]

@rossPatton I can think of situations where one would want to enforce Stylus requirements but not those same rules on things like CSS classes and IDs. I might use Bootstrap to do my layout styling, BEM to define blocks of custom CSS and lowercase-underscore for my actual Stylus naming conventions. It would be inconsistent, but Bootstrap is a third-party system which I can't control, and my head is in a different place when I'm using Stylus versus writing pure CSS. At least, that's how I can see it justified. However, I think this might be a conversation for a different ticket.

Given that the Readme specifies the scope of this flag to include classes and IDs, then I'd say that the problem is fixed and this issue can be closed. It solves my original issue (which was the .{$class-name} case).

If I decided I feel strongly about whether Stylint should be checking my CSS classes and IDs, I can make a new ticket for that discussion.

Thanks!