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.

Sign up for GitHub

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

Jump to bottom

Add options: checkLink, checkBlockQuote, checkEmphasis, checkHeader #35

Merged
merged 6 commits into from
Jul 21, 2018
Merged

Add options: checkLink, checkBlockQuote, checkEmphasis, checkHeader #35

merged 6 commits into from
Jul 21, 2018

Conversation

Leko
Copy link
Contributor

@Leko Leko commented Jul 21, 2018

Fixed #33

azu
azu reviewed Jul 21, 2018
View reviewed changes
test/textlintrc-test.js Outdated
@@ -82,6 +82,61 @@ describe(".textlinrc test", function() {
assert(result.messages[1].column === 8);
});
});
it("should not visit Link, Image, BlockQuote, Emphasis by default", async function() {
Copy link
Member

Choose a reason for hiding this comment

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

test/textlintrc-test.js is legacy style test.
It can be moved to test/prh-rule-tester-test.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved legacy style tests to test/prh-rule-tester-test.js.

@Leko
Copy link
Contributor Author

Leko commented Jul 21, 2018

@azu Maybe, Image is meaningless.
For example, ![ＪＱＵＥＲＹ](https://example.com) is valid always (even if checkImage is true).

Because Image has not any children of Str.

markdown: ![ＪＱＵＥＲＹ](https://example.com "ＪＱＵＥＲＹ")
ast:

{
  "type": "Image",
  "title": "ＪＱＵＥＲＹ",
  "url": "https://example.com",
  "alt": "ＪＱＵＥＲＹ",
  "loc": {
    "start": {
      "line": 1,
      "column": 0
    },
    "end": {
      "line": 1,
      "column": 39
    }
  },
  "range": [
    0,
    39
  ],
  "raw": "![ＪＱＵＥＲＹ](https://example.com \"ＪＱＵＥＲＹ\")"
}

So we can remove checkImage option and Image type from the blacklist, right?

@azu
Copy link
Member

azu commented Jul 21, 2018

@Leko You're correct.
Current Image handling is incorrect.

We should file it as another issue about Image and Code support.

Leko added 2 commits July 21, 2018 22:44
azu
azu approved these changes Jul 21, 2018
View reviewed changes
Copy link
Member

@azu azu left a comment

Choose a reason for hiding this comment

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

LGTM

@azu azu changed the title Add options: checkLink, checkImage, checkBlockQuote, checkEmphasis, checkHeader Add options: checkLink, checkBlockQuote, checkEmphasis, checkHeader Jul 21, 2018
@azu azu merged commit ead3b2a into textlint-rule:master Jul 21, 2018
@azu azu mentioned this pull request Jul 21, 2018
Open
@Leko Leko deleted the add-check-node-type-options branch July 21, 2018 14:12
@azu azu mentioned this pull request Jul 21, 2018
Closed
@azu
Copy link
Member

azu commented Jul 21, 2018

Release https://github.com/textlint-rule/textlint-rule-prh/releases/tag/5.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azu azu azu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Leko @azu