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

Add support for external rules #14

Closed
zcei opened this issue Jun 12, 2015 · 11 comments · Fixed by #15
Closed

Add support for external rules #14

zcei opened this issue Jun 12, 2015 · 11 comments · Fixed by #15
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have

Comments

@zcei
Copy link
Contributor

zcei commented Jun 12, 2015

With this I could build standard-readme as a semantic rule checked by mdast-lint.

I think about smth like this:

{
  "standard-readme": true
}

or even with customization:

{
  "standard-readme": {
    "optional-blocks": ["badges", "reference"]
    "order": ["heading", "badge", "description"]
  }
}
@wooorm
Copy link
Member

wooorm commented Jun 12, 2015

First: loading extra rules wouldn't be very hard. I’ll just require('standard-readme'), and expect it to expose an object of ruleId’s to rules.

Then, every rule could define the options it accepts.

So, to me, something like this fictional .mdastrc could work:

{
  "plugins": {
    "lint": {
      "external": [
        "standard-markdown"
      ],
      "maximum-line-length": 60,
      "some-standard-markdown-rule": false
    }
  }
}

But an alternative would be the following:

{
  "plugins": {
    "lint": {
      "external": {
        "standard-markdown": {
          "some-standard-markdown-rule": false
        }
      },
      "maximum-line-length": 60
    }
  }
}

However, this setup (just a rule object as exports) would not support plug-in specific settings...

@zcei
Copy link
Contributor Author

zcei commented Jun 12, 2015

I would opt for the first version, as same names imply same rule.
At edge-cases we can still prefix.

So every external rule is configurable at the level where lint rules currently are.
For me this makes most sense, and keeps the nesting kinda flat.

Is there currently anything that restricts plugin options to bool/string?
Following would be nice as customization to external rules:

{
  "plugins": {
    "lint": {
      "external": ["standard-markdown"],
      "maximum-line-length": 60,
      "standard-markdown-optional-blocks": ["badges", "reference"]
    }
  }
}

@wooorm
Copy link
Member

wooorm commented Jun 12, 2015

Rules can receive anything. Only the settings relating to reset (null, undefined, false, and true) have an extra meaning (hiding or showing the messages).

Still, as i explained earlier, all rules are run regardless of value, and receive that value as a third argument.

@wooorm
Copy link
Member

wooorm commented Jun 12, 2015

I agree btw that your example looks good :)

@wooorm
Copy link
Member

wooorm commented Jun 12, 2015

@zcei Hmm, I’m still missing one part, how about the JS API, something like this?

var mdast = require('mdast');
var lint = require('mdast-lint');
var standard = require('mdast-lint-standard');

var processor = mdast().use(lint, {
  'external': [standard],
  'maximum-line-length': 60,
  'standard-markdown-optional-blocks': ['badges', 'reference']
});

@zcei
Copy link
Contributor Author

zcei commented Jun 12, 2015

I'd rather have the plugin configured before:

var mdast = require('mdast');
var lint = require('mdast-lint');
var standard = require('mdast-lint-standard');

standard.optionalBlocks(['badges', 'reference']);

var processor = mdast().use(lint, {
  'external': [standard],
  'maximum-line-length': 60
});

Then you can easily reuse some parts without fuzzy work with removing the configuration values from the object.
For the .mdastrc (not sure whether we need yet another .*rc) it's more "long-living" than for code, so for me it's better to stick to our consensus from above there.

But then, having two different styles in .mdastrc and API is kinda ugly.

Thoughts on this?

@zcei
Copy link
Contributor Author

zcei commented Jun 12, 2015

To be consistent in the example:

var mdast = require('mdast');
var lint = require('mdast-lint');
var standard = require('mdast-lint-standard');

standard('optional-blocks', ['badges', 'reference']);
standard.optionalBlocks(['badges', 'reference']);

lint('maximum-line-length', 60);
lint.maximumLineLength(60);

lint('external', [standard]);
lint.external([standard]);

var processor = mdast().use(lint);

@wooorm
Copy link
Member

wooorm commented Jun 12, 2015

TBH I’d rather not add much difference to the CLI/API. Especially the boilerplate needed for many getters/setters seems unnecessary to me!

In addition, the above setters all set options on the global standard object, which wouldn't work well for different settings for different files, and when other tools use standard.

Why not just use JSON when possible for settings?

@zcei
Copy link
Contributor Author

zcei commented Jun 12, 2015

Guess you're right - there is no negative argument I could provide against your provided solution.

@wooorm
Copy link
Member

wooorm commented Jun 12, 2015

Alright :) however, it's your plugin, so you can do whatever! But this system is already in place so it'd be less of a hassle!

@zcei
Copy link
Contributor Author

zcei commented Jun 12, 2015

Err.. what kind of community are we in here? :)
Your arguments are reasonable, and in no intention harmful to me, so of course I'm interested in what others think about.

And due to its intention to serve as a standard for the community there is no "do whatever you want", more like "do whatever everyone wants" 🎯

wooorm added a commit that referenced this issue Jun 12, 2015
*   Add example rule, and rule object file, to
    `test/external`;
*   Add fixtures for external tests;
*   Move `mdast.js`, `mdast.min.js` to `mdast-lint.js`,
    `mdast-lint.min.js`;
*   Add docs for external rules.

Closes GH-14.
wooorm added a commit that referenced this issue Jun 13, 2015
*   Add example rule, and rule object file, to
    `test/external`;
*   Add fixtures for external tests;
*   Move `mdast.js`, `mdast.min.js` to `mdast-lint.js`,
    `mdast-lint.min.js`;
*   Add docs for external rules.

Closes GH-14.
@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface 🦋 type/enhancement This is great to have 🧒 semver/minor This is backwards-compatible change labels Aug 15, 2019
@wooorm wooorm added the 💪 phase/solved Post is done label Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging a pull request may close this issue.

2 participants