-
Notifications
You must be signed in to change notification settings - Fork 15
Conversation
c0f3d2d
to
0881382
Compare
0881382
to
1100406
Compare
Thanks @obenland. Do you think I should start a new issue to discuss which rules we want to include? |
I think we can do that here and then add those in |
Cool. Thinking we should actually be pretty conservative about which rules we're including. I'm thinking... |
@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. |
@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? |
I followed the instructions here, removed the unwanted rule files from |
…ting into add/csslint-customization
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 Thoughts? There are still a few warnings shown in the |
better-code-editing.php
Outdated
@@ -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? |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
I'm working on the refactor to add the |
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.