Add stylelint integration by westonruter · Pull Request #239 · xwp/wp-dev-lib

westonruter · 2017-05-17T07:01:48Z

Fixes #47

ntwb

Awesome @westonruter, I've added numerous inline comments 👍

ntwb · 2017-05-17T09:02:18Z

check-diff.sh


+		# Install stylelint
+		if [ -n "$STYLELINT_CONFIG" ] && [ -e "$STYLELINT_CONFIG" ] && [ ! -e "$(npm bin)/stylelint" ] && check_should_execute 'stylelint'; then
+			echo "Installing ESLint"


Should be echo "Installing stylelint"

ntwb · 2017-05-17T09:02:51Z

check-diff.sh

+		# Install stylelint
+		if [ -n "$STYLELINT_CONFIG" ] && [ -e "$STYLELINT_CONFIG" ] && [ ! -e "$(npm bin)/stylelint" ] && check_should_execute 'stylelint'; then
+			echo "Installing ESLint"
+			if ! npm install -g eslint 2>/dev/null; then


Should be if ! npm install -g stylelint 2>/dev/null; then

ntwb · 2017-05-17T09:13:24Z

check-diff.sh

+	if [ -z "$STYLELINT_CONFIG" ]; then
+		STYLELINT_CONFIG="$( upsearch stylelint.config.js )"
+	fi
+


stylelint supports a few more config files, not sure if you want to include all possible combinations here

Via https://stylelint.io/user-guide/configuration/
The .stylelintrc file (without extension) can be in JSON or YAML format. Alternately, you can add a filename extension to designate JSON, YAML, or JS format: .stylelintrc.json, .stylelintrc.yaml, .stylelintrc.yml, .stylelintrc.js. You may want to use an extension so that your text editor can better interpret the file, and help with syntax checking and highlighting.

ntwb · 2017-05-17T09:24:32Z

check-diff.sh

 	cat "$TEMP_DIRECTORY/paths-scope" | grep -E '\.php(:|$)' | cat - > "$TEMP_DIRECTORY/paths-scope-php"
 	cat "$TEMP_DIRECTORY/paths-scope" | grep -E '\.jsx?(:|$)' | cat - > "$TEMP_DIRECTORY/paths-scope-js"
-	cat "$TEMP_DIRECTORY/paths-scope" | grep -E '\.(css|scss)(:|$)' | cat - > "$TEMP_DIRECTORY/paths-scope-scss"
+	cat "$TEMP_DIRECTORY/paths-scope" | grep -E '\.(css|scss)(:|$)' | cat - > "$TEMP_DIRECTORY/paths-scope-css"


Let's drop scss from the above and only go with css initially.

The stylelint-config-wordpress configuration includes two stylelint shared configs, one for CSS and one for SCSS, so let's get CSS up and running and follow up with with SCSS in another PR

ntwb · 2017-05-17T09:25:27Z

check-diff.sh


 		# Make sure linter configs get copied linting directory since upsearch is relative.
-		for linter_file in .jshintrc .jshintignore .jscsrc .jscs.json .eslintignore .eslintrc phpcs.ruleset.xml ruleset.xml; do
+		for linter_file in .jshintrc .jshintignore .jscsrc .jscs.json .eslintignore .eslintrc .stylelintrc stylelint.config.js phpcs.ruleset.xml ruleset.xml; do


See also above comment on adding all possibilities of valid stylelint configuration file names.

ntwb · 2017-05-17T09:27:36Z

check-diff.sh

+			echo "## stylelint"
+			cd "$LINTING_DIRECTORY"
+			# TODO: --format=compact is not right.
+			if ! cat "$TEMP_DIRECTORY/paths-scope-css" | remove_diff_range | xargs "$(npm bin)/stylelint" --format=compact --config="$STYLELINT_CONFIG" > "$TEMP_DIRECTORY/stylelint-report"; then


Rather than --format=compact, it should be --custom-formatter stylelint-formatter-compact

It seems that it actually needs to be --custom-formatter node_modules/stylelint-formatter-compact

ntwb · 2017-05-17T09:28:50Z

pre-commit

 cat "$TEMP_DIRECTORY/paths-scope"

 check_execute_bit
+lint_css_files


As noted above, in a follow up PR we can add lint_scss_files

ntwb · 2017-05-17T09:29:42Z

readme.md

 ```

-For ESLint, you'll also likely want to make `eslint` as a dev dependency for your NPM package:
+For ESLint and stylelint, you'll also likely want to make then dev dependencies for your NPM package:


them, not then

westonruter · 2017-05-17T18:54:38Z

Example working integration: xwp/wp-core-media-widgets#183

westonruter · 2017-05-17T22:32:27Z

check-diff.sh

+		(
+			echo "## stylelint"
+			cd "$LINTING_DIRECTORY"
+			if ! cat "$TEMP_DIRECTORY/paths-scope-css" | remove_diff_range | xargs "$(npm bin)/stylelint" --custom-formatter=node_modules/stylelint-formatter-compact --config="$STYLELINT_CONFIG" > "$TEMP_DIRECTORY/stylelint-report"; then


Not sure about the node_modules/stylelint-formatter-compact part. What if stylelint-formatter-compact is installed globally?

Swap the = for a space , i.e: --custom-formatter node_modules/stylelint-formatter-compact

Ignore that ^^, the = does not matter

It doesn't make a difference. Both --custom-formatter node_modules/stylelint-formatter-compact and --custom-formatter=node_modules/stylelint-formatter-compact have the same result.

Also, both --custom-formatter stylelint-formatter-compact and --custom-formatter=stylelint-formatter-compact fail with: “Error: Cannot find module”.

ntwb · 2017-05-17T23:01:36Z

.editorconfig

 indent_style = tab

-[{package.json,*.yml}]
+[{*.json,*.yml,.stylelintrc,.jshintrc,.eslintrc,.jscsrc}]


Also *.yaml

ntwb · 2017-05-17T23:08:33Z

check-diff.sh

+		(
+			echo "## stylelint"
+			cd "$LINTING_DIRECTORY"
+			if ! cat "$TEMP_DIRECTORY/paths-scope-css" | remove_diff_range | xargs "$(npm bin)/stylelint" --custom-formatter=node_modules/stylelint-formatter-compact --config="$STYLELINT_CONFIG" > "$TEMP_DIRECTORY/stylelint-report"; then


Swap the = for a space , i.e: --custom-formatter node_modules/stylelint-formatter-compact

ntwb · 2017-07-24T06:46:48Z

.editorconfig

 indent_style = tab

-[{package.json,*.yml}]
+[{*.json,*.yml,*.yaml,.stylelintrc,.jshintrc,.eslintrc,.jscsrc}]


This change is no longer needed, all .json files should now use tabs for indentation

See https://core.trac.wordpress.org/ticket/40946

…tylelint

…onfigs" This partially reverts commit b0eff2f.

Add WIP for stylelint integration

a3bd485

westonruter mentioned this pull request May 17, 2017

Add support for CSS validation #47

Open

ntwb suggested changes May 17, 2017

View reviewed changes

westonruter added 4 commits May 17, 2017 11:02

Support all stylelint config file formats

3933f30

Omit SCSS from CSS manifest

bcdb366

Fix references to stylelint from eslint

2b06eed

Fix passing of custom-formatter

73fdbd2

westonruter changed the title ~~[WIP] Add stylelint integration~~ Add stylelint integration May 17, 2017

westonruter mentioned this pull request May 17, 2017

Add stylelint xwp/wp-core-media-widgets#183

Open

Update .editorconfig with latest from core with also linter configs

b0eff2f

westonruter commented May 17, 2017

View reviewed changes

ntwb reviewed May 17, 2017

View reviewed changes

westonruter added 2 commits May 17, 2017 16:18

Include *.yaml in .editorconfig

a23b89b

Fix comment

580e388

ntwb reviewed Jul 24, 2017

View reviewed changes

westonruter added 2 commits July 24, 2017 13:06

Merge branch 'master' of https://github.com/xwp/wp-dev-lib into add/s…

2259c48

…tylelint

Revert "Update .editorconfig with latest from core with also linter c…

c2d14a0

…onfigs" This partially reverts commit b0eff2f.

ntwb mentioned this pull request Jul 25, 2017

General FE Workflow [DO NOT MERGE] #245

Closed

Conversation

westonruter commented May 17, 2017

Uh oh!

ntwb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter commented May 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants