textlint-rule / textlint-rule-prh

textlint rule for prh.
MIT License
82 stars 6 forks source link

Proposal: Add options to customize filter AST type #33

Closed Leko closed 6 years ago

Leko commented 6 years ago

Currently, filtered AST types are Link, Image, BlockQuote, Emphasis. https://github.com/textlint-rule/textlint-rule-prh/blob/dd4bb87109a10ed428eee8baee76925b6396c032/src/textlint-rule-prh.js#L136

I want to add Header to the blacklist. But it cannot set as default behavior. It's not needed in many use cases. So I propose to support ignoreSyntax option.

How about it?

azu commented 6 years ago

Thanks for report.

tl;dr: ignoreSyntax name is large in scope for the behavior.

This option is reasonable. But, we need to get more meaningful naming instead of ignoreSyntax. Because, following option mismatch the behavior.

{
  "prh": {
    "ignoreSyntax": []
  }
}

It seems that prh check all syntax that includes Code. But, the expectation mismatch real behavior. Actually, texltint-rule-prh does not check Code. (I want to check Code node by option in the future.)

Note: use NodeType instead of Syntax.

Leko commented 6 years ago

@azu Then, what about checkHeader option?

Naming rule: {prefix}{NodeType}

Other possibilities of prefix:

azu commented 6 years ago

I like visitHeader, but it is non-intuitive naming. I think that checkHeader is passable.

Leko commented 6 years ago

I think so. visit brings to mind internal behavior. Gotcha. I'll submit a pull request named as check*. Thanks.