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

Import textlint-plugin-html to monorepo #364

Closed
wants to merge 33 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@Leko
Member

Leko commented Dec 11, 2017

This PR is partial of #270

azu and others added some commits Oct 29, 2015

fix(plugin): Add url property to Link Node (#8)
* Add url property to Link Node

When applying `textlint-rule-no-dead-link` to HTML, an error occurred.

`` `console
✖ Stack trace
TypeError: Parameter "url" must be a string, not undefined
    at Url.parse (url.js: 102: 11)
    at Object.urlParse [as parse] (url.js: 96: 5)
    at isRelative
(/usr/local/lib/node_modules/textlint-rule-no-dead-link/lib/no-dead-link
.js:83:24)
    at
/usr/local/lib/node_modules/textlint-rule-no-dead-link/lib/no-dead-link.
js: 107: 11
    at Generator.next (<anonymous>)
`` `

textlint-rule-no-dead-link expects the Link Node to hold the url
property, and in the case of txt, markdown it holds the actual url
property.
I think that the Link Node generated from the html file should also
keep the url property. [^ 1]

[1]: The href attribute of `a` tag is not mandatory.
If href does not exist, url is not set.
If url does not exist, a fix to ignore the target Link node is required
for `textlint-rule-no-dead-link`.

* Fix undefined check of href, and add test for placeholder.

Fix undefined check of href, and add test for placeholder.
@azu

azu requested changes Dec 11, 2017 edited

Thanks for PR.

It need to core maintainer I think.

Currenly, textlint-plugin-html is experimental implementation.
It has a risk that impot this plugin into the monorepo.

@@ -474,6 +474,7 @@ These modules are parts of textlint.
| [`textlint-formatter`](/packages/textlint-formatter) | [![npm](https://img.shields.io/npm/v/textlint-formatter.svg?style=flat-square)](https://www.npmjs.com/package/textlint-formatter) | textlint output formatter |
| [`textlint-plugin-markdown`](/packages/textlint-plugin-markdown) | [![npm](https://img.shields.io/npm/v/textlint-plugin-markdown.svg?style=flat-square)](https://www.npmjs.com/package/textlint-plugin-markdown) | markdown support for textlint |
| [`textlint-plugin-text`](/packages/textlint-plugin-text) | [![npm](https://img.shields.io/npm/v/textlint-plugin-text.svg?style=flat-square)](https://www.npmjs.com/package/textlint-plugin-text) | plain text support for textlint |
| [`textlint-plugin-html`](/packages/textlint-plugin-html) | [![npm](https://img.shields.io/npm/v/textlint-plugin-html.svg?style=flat-square)](https://www.npmjs.com/package/textlint-plugin-html) | HTML support for textlint |

This comment has been minimized.

@azu

azu Dec 11, 2017

Member

It is not a core package.
textlint-plugin-html is not built-in plugin in textlint.
That is a reason that I've noted "secondary" at #270

I'm careful what to import this plugin into monorepo.
Because, I'm not a html plugin user, and textlint-plugin-html is not stable plugin textlint/textlint-plugin-html#2 (version < 1.0.0)

If core maintainer exists for html plugin, I'm welcome to import textlint-plugin-html into repository.

@kemsakurai Are you interested in maintaining textlint-plugin-html?

@azu

azu Dec 11, 2017

Member

It is not a core package.
textlint-plugin-html is not built-in plugin in textlint.
That is a reason that I've noted "secondary" at #270

I'm careful what to import this plugin into monorepo.
Because, I'm not a html plugin user, and textlint-plugin-html is not stable plugin textlint/textlint-plugin-html#2 (version < 1.0.0)

If core maintainer exists for html plugin, I'm welcome to import textlint-plugin-html into repository.

@kemsakurai Are you interested in maintaining textlint-plugin-html?

This comment has been minimized.

@Leko

Leko Dec 11, 2017

Member

Oops, I make a mistake.
I'm sorry for submit PR without sufficient understanding this repository.

@Leko

Leko Dec 11, 2017

Member

Oops, I make a mistake.
I'm sorry for submit PR without sufficient understanding this repository.

This comment has been minimized.

@azu

azu Dec 11, 2017

Member

@Leko Don't worry. It's my mistake.

@azu

azu Dec 11, 2017

Member

@Leko Don't worry. It's my mistake.

@azu azu referenced this pull request Dec 11, 2017

Open

Import * to monorepo #270

11 of 14 tasks complete

@azu azu added the Status: Blocked label Dec 12, 2017

@azu

This comment has been minimized.

Show comment
Hide comment
@azu

azu Aug 8, 2018

Member

This PR will be closed.
Because, original repository is updated.
textlint/textlint-plugin-html#9

We need to import the diff if merge this PR.

Member

azu commented Aug 8, 2018

This PR will be closed.
Because, original repository is updated.
textlint/textlint-plugin-html#9

We need to import the diff if merge this PR.

@azu azu closed this Aug 8, 2018

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