search

tani / textlint-rule-spelling

A textlint rule for spellings of languages as much as possible
GNU General Public License v3.0
8 stars 5 forks source link

Apply global flag to wordDefinitionRegexp #10

Closed kimburgess closed 3 years ago

kimburgess commented 3 years ago

Currently when configuring from .textlintrc there is no way to specify a wordDefinitionRegexp that will match globally. The RegExp constructor does not accept flags as part of a string based pattern.

Including the global flag seems to be a sensible default that works around this.

tani commented 3 years ago

Thanks! Could you add tests satisfying the following condition for maintaining this rule?

kimburgess commented 3 years ago

Certainly. This was a lazy in-browser PR - thank you for being thorough and requesting coverage. Will pop an update through later today.

tani commented 3 years ago

Thank you for adding new tests. I run the test in my computer. I could pass tests if I give "g" flag to this rule. However, I stuck tests, and they run forever if we remove "g" flag. It is not fail, it runs forever. Could you explain this error?

kimburgess commented 3 years ago

That is due to this loop behaviour: https://github.com/tani/textlint-rule-spelling/blob/68eb8105d3179625e1bc7c5df6cc550dc806fe56/src/index.js#L104-L108

Without the global flag, the RegExp object is not stateful. The result of this is the exec will match indefinitely on the first word, never exhaust the input and remain in that loop until the heat death of the universe (or you SIGINT, whichever comes first).

The current structure assumes / requires stateful regexp objects, which is what this PR enforces :)

tani commented 3 years ago

Thank you for explanations and great contribution. I merge it. Cheers :beers:

tani commented 3 years ago

@KimBurgess I have published v0.3.0 on npm. Please check it.

kimburgess commented 3 years ago

Perfect. That works!