Skip to content
This repository has been archived by the owner on Dec 11, 2017. It is now read-only.

Add a way to customize CSSLint rules #38

Merged
merged 10 commits into from
Aug 26, 2017
Merged

Conversation

obenland
Copy link
Member

@obenland obenland commented Aug 21, 2017

Maybe this should be part of a bigger discussion about finding a way to customize certain parts of CodeMirror. I was going back and forth between individual customization files like I'm proposing in this PR, and one combined file, something like wp-codemirror.js or something.

We should also go through the rules CSSLint comes with and see which ones we want to keep or not. /cc @melchoyce

Fixes #26.

@obenland obenland force-pushed the add/csslint-customization branch from c0f3d2d to 0881382 Compare August 21, 2017 16:05
@obenland obenland force-pushed the add/csslint-customization branch from 0881382 to 1100406 Compare August 21, 2017 16:44
@melchoyce
Copy link
Collaborator

Thanks @obenland. Do you think I should start a new issue to discuss which rules we want to include?

@obenland
Copy link
Member Author

I think we can do that here and then add those in

@melchoyce
Copy link
Collaborator

melchoyce commented Aug 21, 2017

Cool. Thinking we should actually be pretty conservative about which rules we're including. I'm thinking...

@obenland
Copy link
Member Author

@westonruter Given how many rules we remove here, I wonder if it would be better to create a custom build of csslint than to have the default script add all the rules and us the overwriting them later on? I'd have one ready to go.

@westonruter
Copy link
Member

@obenland My only concern there would be if someone would like to enable those rules via filtering, but if they aren't part of the build then they wouldn't be able to do it. How is a custom build made?

@obenland
Copy link
Member Author

How is a custom build made?

I followed the instructions here, removed the unwanted rule files from src/rules, and ran grunt concat. The resulting file can then be found in dist.

@westonruter
Copy link
Member

I'm trying an alternate approach to defining the rules in af724cc. This could allow us to use PHP to filter the rules that the user wants. It's a bit hacky in how it CSSLint doesn't have a removeRule method, so it has to clear all of them and then re-add the ones that we want.

Thoughts?

There are still a few warnings shown in the style.css for Twenty Seventeen, but there are far fewer than before.

@@ -158,6 +158,27 @@ public static function prep_codemirror_for_file( $file ) {
wp_enqueue_style( 'codemirror' );
wp_enqueue_style( 'codemirror-addon-lint' );

// @todo Let this be filterable?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could pass the list of rules in the options object that gets passed to CodeMirror, but we could have a wp-code-editor wrapper script, with a wp.codeEditor.initialize( id, options ) wrapper that emulates wp.editor.initialize() for TinyMCE. The wp.codeEditor.initialize function could grab additional properties in options, like csslint-rules, and then configure CSSLint before initializing CodeMirror since the CodeMirror options doesn't allow for the list of rules to be defined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we had a wp.codeEditor as well, this could provide an abstraction which would de-couple us from having to specifically support CodeMirror directly, and we could in the future switch to a different editor if we need to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And back to the note about being filterable, this could be part of the $options array which then later gets filtered theme_editor_codemirror_opts or plugin_editor_codemirror_opts, both of which could be simply a wp_code_editor_settings filter for parity with TinyMCE's wp_editor_settings filter.

@westonruter
Copy link
Member

I'm working on the refactor to add the wp.codeEditor abstraction.

@westonruter westonruter merged commit 533bd7a into master Aug 26, 2017
@westonruter westonruter deleted the add/csslint-customization branch August 28, 2017 22:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants