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

Complains about file names in text #2

Closed
sapegin opened this Issue Dec 11, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@sapegin

sapegin commented Dec 11, 2017

Text:

# Anatomy of a Package

A minimal npm package should contain metadata in a _package.json_ file and an associated source file (usually _index.js_). In practice, packages contain more than that and you will have at least a license file and the source in various formats.

Result:

   3:61   ✓ error  Heading: Follow the standard capitalization rules for American English.
See https://owl.english.purdue.edu/owl/resource/592/01/  en-capitalization
   3:118  ✓ error  Heading: Follow the standard capitalization rules for American English.
See https://owl.english.purdue.edu/owl/resource/592/01/  en-capitalization

Not sure it’s a bug or a feature ;-)

@azu azu added the Type: Bug label Dec 11, 2017

@azu

This comment has been minimized.

Show comment
Hide comment
@azu

azu Dec 11, 2017

Member

It is a bug.
Fmm. This issue come from sentence-splitter. sentence-splitter just split text by .

const source = new StringSource(node);
// Textlint Node to Plaing Text
const sourceText = source.toString();
// Split Plain text by `.`
const sentences = split(sourceText);

const source = new StringSource(node);
const sourceText = source.toString();
const sentences = split(sourceText);

As a result, it split _package.json_ to ["package", "json"] sentences.(This is wrong sentences)

We need to more conventional sentence-splitter for textlint nodes...
(Paragraph Node to Sentence Nodes.)

📝 Workaround resolution, fill Emphasis node's value with dummy value like _package.json_ => _XXXXXXXXX_.

Member

azu commented Dec 11, 2017

It is a bug.
Fmm. This issue come from sentence-splitter. sentence-splitter just split text by .

const source = new StringSource(node);
// Textlint Node to Plaing Text
const sourceText = source.toString();
// Split Plain text by `.`
const sentences = split(sourceText);

const source = new StringSource(node);
const sourceText = source.toString();
const sentences = split(sourceText);

As a result, it split _package.json_ to ["package", "json"] sentences.(This is wrong sentences)

We need to more conventional sentence-splitter for textlint nodes...
(Paragraph Node to Sentence Nodes.)

📝 Workaround resolution, fill Emphasis node's value with dummy value like _package.json_ => _XXXXXXXXX_.

@sapegin

This comment has been minimized.

Show comment
Hide comment
@sapegin

sapegin Dec 11, 2017

This issue come from sentence-splitter. sentence-splitter just split text by .

Shouldn’t it require non-alphabetic character after .?

sapegin commented Dec 11, 2017

This issue come from sentence-splitter. sentence-splitter just split text by .

Shouldn’t it require non-alphabetic character after .?

@azu

This comment has been minimized.

Show comment
Hide comment
@azu

azu Dec 11, 2017

Member

Shouldn’t it require non-alphabetic character after .?

Yes. You're correct.
But, current sentence-splitter implementation is buggy. 🐛

Member

azu commented Dec 11, 2017

Shouldn’t it require non-alphabetic character after .?

Yes. You're correct.
But, current sentence-splitter implementation is buggy. 🐛

azu added a commit that referenced this issue Dec 24, 2017

@azu azu closed this in #3 Dec 24, 2017

azu added a commit that referenced this issue Dec 24, 2017

fix(rule): fix false-positive result (#3)
* fix(rule): fix false-positive result

Completely rewrite sentence parser https://github.com/azu/sentence-splitter/releases/tag/3.0.0

fix #2

* chore(npm): remove unused library
@azu

This comment has been minimized.

Show comment
Hide comment
@azu

azu Dec 24, 2017

Member

I've completely rewrited sentence splitter.

And, I've upgraded this rule to use latest sentence-splitter #3

Member

azu commented Dec 24, 2017

I've completely rewrited sentence splitter.

And, I've upgraded this rule to use latest sentence-splitter #3

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