postcss / postcss-bem-linter

A BEM linter for postcss
MIT License
572 stars 35 forks source link

working with postcss-cli #55

Closed VinSpee closed 9 years ago

VinSpee commented 9 years ago

Is this configuration compatible with postcss-cli? It seems as though it's not. when doing:

postcss --use postcss-bem-linter --postcss-bem-linter.namespace "br" ./src/styles/main.css

I'm getting the error

[TypeError: Cannot read property 'initial' of undefined]

I assume it's because of this's plugin's unusual configuration signature - any thoughts?

davidtheclark commented 9 years ago

What's unusual about the signature? Is it that there are two arguments instead of just one object?

VinSpee commented 9 years ago

yes, that's what I am referring to. What are your thoughts on it?

davidtheclark commented 9 years ago

I guess I don't see a reason not to merge those two arguments into one big options object except that it would mean a breaking change. I haven't looked into postcss-cli yet --- I'll take a gander.

VinSpee commented 9 years ago

Yeah that's my worry too. Seems like a waste to do a 2.0 when you're just moving an argument. Then again, it might be worth it to follow "convention"-ish? And versions are free. On Thu, Jul 16, 2015 at 19:11 David Clark notifications@github.com wrote:

I guess I don't see a reason not to merge those two arguments into one big options object except that it would mean a breaking change. I haven't looked into postcss-cli yet --- I'll take a gander.

— Reply to this email directly or view it on GitHub https://github.com/postcss/postcss-bem-linter/issues/55#issuecomment-122127579 .

VinSpee commented 9 years ago

looks like it's in the postcss plugin guidelines

davidtheclark commented 9 years ago

We're using the plugin function here, so I don't think it's violating the guidelines.

I try to look into this and the CLI this weekend sometime.

davidtheclark commented 9 years ago

Sorry I've been dilatory on the update, @VinSpee. Here's what I'm thinking: I want to make some other breaking changes to the API -- leaving the :root checking stuff to stylelint. But I wanted to wait until https://github.com/stylelint/stylelint/issues/133 is taken care of. I'd then work this breaking change into the same release. Seem good?

VinSpee commented 9 years ago

Makes great sense - thanks On Wed, Jul 29, 2015 at 22:56 David Clark notifications@github.com wrote:

Sorry I've been dilatory on the update, @VinSpee https://github.com/VinSpee. Here's what I'm thinking: I want to make some other breaking changes to the API -- leaving the :root checking stuff to stylelint. But I wanted to wait until stylelint/stylelint#133 https://github.com/stylelint/stylelint/issues/133 is taken care of. I'd then work this breaking change into the same release. Seem good?

— Reply to this email directly or view it on GitHub https://github.com/postcss/postcss-bem-linter/issues/55#issuecomment-126167385 .

davidtheclark commented 9 years ago

@VinSpee: I thought some more about this and left a comment in postcss-cli: https://github.com/code42day/postcss-cli/issues/21#issuecomment-127816623

I'm not very inclined to shape the API to accommodate the limitations of postcss-cli's --plugin.option format. If we think of an arrangement that seems better for its own sake, though, sure.

davidtheclark commented 9 years ago

What do you think of this possible accommodation?

You can use a preset pattern and its options in two ways:
- pass the preset's name as the `pattern`, and, if needed, an `options` object,
  e.g. `bemLinter('suit', { namespace: 'twt' })`.
- pass an object as `pattern`, with the preset's name as the `preset` property 
  and that preset's options as other properties, 
  e.g. `bemLinter({ preset: 'suit', namespace: 'twt' })`.
davidtheclark commented 9 years ago

Any @postcss/owners have opinions about this kind of function signature business and whether/how to accommodate postcss-cli?

davidtheclark commented 9 years ago

Or another possibility: bemLinter({ preset: 'suit', presetOptions: { namespace: 'twt' }})

davidtheclark commented 9 years ago

Just released 0.5.0 allowing for that last idea: bemLinter({ preset: 'suit', presetOptions: { namespace: 'twt' }})

VinSpee commented 9 years ago

👌🏼Thanks!

ben-eb commented 9 years ago

@davidtheclark Late to the party but yeah, 0.5.0 lgtm. :+1: