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

No error prompt on closing curly brace in CSS section #191

Closed
jwang1919 opened this issue Oct 5, 2016 · 6 comments
Closed

No error prompt on closing curly brace in CSS section #191

jwang1919 opened this issue Oct 5, 2016 · 6 comments
Assignees
Labels

Comments

@jwang1919
Copy link
Contributor

Popcode will read the following CSS as valid with no error prompt.
p { color: black;

@outoftime
Copy link
Contributor

I think the fix for this is to incorporate stylelint, which does report this as an error.

@outoftime
Copy link
Contributor

@jwang1919 this would be useful and possibly juicy and fun if you are looking for a meaty contribution!

@jwang1919
Copy link
Contributor Author

I'll work on it the week after. I got other commitments this weekend 😄 .

@jwang1919
Copy link
Contributor Author

@outoftime please assign this to me. I can't find any option to do so myself. I'll start doing some basic digging into this issue.

@outoftime
Copy link
Contributor

@jwang1919 all yours!!

jwang1919 added a commit to jwang1919/popcode that referenced this issue Nov 12, 2016
jwang1919 added a commit to jwang1919/popcode that referenced this issue Nov 13, 2016
jwang1919 added a commit to jwang1919/popcode that referenced this issue Nov 13, 2016
outoftime added a commit to jwang1919/popcode that referenced this issue Nov 18, 2016
outoftime pushed a commit that referenced this issue Nov 18, 2016
* Initial commit. Added stylelint to npm package. Still getting it to play nicely with webpack.

* Still broken. Webpack seems to be unable to load stylelint properly, or I'm doing something wrong.

* Browser-friendly stylelint build

The default `stylelint` module wants to load configuration files from
the filesystem in a dynamic way that isn’t possible with `brfs`. We
don’t actually need the file-loading behavior, so instead, we hand-roll
a `stylelint` object in the same manner as
[`createStylelint`](https://github.com/stylelint/stylelint/blob/7.5.0/src/createStylelint.js),
but using configuration defined in code rather than the filesystem. This
definitely builds in knowledge of some non-public APIs in `stylelint`,
but I can’t think of a better approach.

Also a decent amount of Webpack wrangling to get the modules we do
import from stylelint to build. In particular, they *still* want to hit
the filesystem, although they don’t actually need to in order to work
for us. So, instead of building those modules with `brfs`, just use the
string replace loader to turn `require("fs")` into a null object; use
the `substitute` loader to stub out the `isAutoprefixable` method; and
use the null module loader to stub out rules that want access to a
gigantic database of browser capabilities. The usage of
string-replacement is a fragile hack, but I can’t think of another way
to *only* stub out `fs` in that particular context.

* Fix npm test errors after merge from outoftime's browser-friendly
stylelint build

* Working stylelint build with validations

* Merged from master branch

* Refactored stylelint.js. Fixed _locationForError function so it's working properly.

* Changed code to follow PR#514 suggestions.
@outoftime
Copy link
Contributor

Fixed by #556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants