IQTLabs / textlint-rule-one-sentence-per-line

A fixable textlint rule to add a line break after each sentence
Apache License 2.0
1 stars 3 forks source link

"#. One sentence." detected as two sentences #10

Open torbsorb opened 3 years ago

torbsorb commented 3 years ago

Combing this rule with the textlint-plugin-rst causes the following to be detected as an error:

#. Ordered list with one sentence.
#. New item in an ordered list, still one sentence.
   This is the second sentence in the second item.

It seems to detect #. as a sentence. It should be allowed to have at least one sentence on the same line as the #. marker.

The following is accepted, renders correctly, but should not be necessary, right?

#. 
   Ordered list with one sentence.
#. New item in an ordered list, still one sentence.
   This is the second sentence in the second item.

The --fix option does not produce this. It does not maintain the required indentation. It produces the following, which does not render correctly:

#. 
Ordered list with one sentence.
#. 
New item in an ordered list, still one sentence.
   This is the second sentence in the second item.
Benjamin-Lee commented 3 years ago

@kimburgess, do you have any idea what's going on with this?

kimburgess commented 3 years ago

I was just looking at this exact item locally.

The following is semantically valid and should parse as a single List containing two ListItem child nodes:

1. This is a list.
It has complex entries.
2. Each may have multiple sentences

Instead, it's splitting into a List with a single ListItem, followed by a Paragraph that includes the second sentence and following item. This is visible within the AST viewer so appears to be an issue in TextLint itself. Just taking a quick look now for a potential cause.

kimburgess commented 3 years ago

The loose specifications around markdown seem to be the cause here. TextLint does correct parse complex list items, but treats whitespace as semantically important as part of this.

1. This is a list.
   It has complex entries.
   Indentation causes them to group corectly.
2. Each may have multiple sentences

AST

This reflects the behaviour defined for GFM (which is a little more tightly specified), so looks to be working likely as per design.

torbsorb commented 3 years ago

Even so, the following should be allowed in the reST file, and not cause an error with textlint-rule-one-sentence-per-line?

#. One sentence.
#. Another sentence.

The problem is that your example causes an error, while the following does not:

1. 
   This is a list.
   It has complex entries.
   Indentation causes them to group correctly.
2. 
   Each may have multiple sentences
kimburgess commented 3 years ago

@torbsorb do you have an example of where this is being a little odd? I've just added test cases for what we were talking about above, but can't generate a failure scenario.

torbsorb commented 3 years ago

Save the following to a one-sentence-per-line.rst file:

The following is a list with a single sentence per item.
This should be allowed.

#. This a list.
#. This continues the list.

Use the following configuration in .textlintrc:

{
  "plugins": [
      "rst"
  ],
  "rules": {
    "one-sentence-per-line": true,
  }
}

Install the reST plugin; textlint-plugin-rst Install textlint-rule-one-sentence-per-line

Run:

textlint .\one-sentence-per-line.rst

I get the following output:

C:\one-sentence-per-line.rst
  4:1  ✓ error  More than one sentence per line  one-sentence-per-line
  5:1  ✓ error  More than one sentence per line  one-sentence-per-line

✖ 2 problems (2 errors, 0 warnings)

If I run with --fix, I get the following incorrect output:

The following is a list with a single sentence per item.
This should be allowed.

#.
This a list.
#.
This continues the list.
torbsorb commented 3 years ago

Just running your example:

1. This is a list.
   It has complex entries.
   Indentation causes them to group corectly.
2. Each may have multiple sentences

--fix yields:

1.
This is a list.
It has complex entries.
   Indentation causes them to group correctly.
2.
Each may have multiple sentences
kimburgess commented 3 years ago

Ah yes, you mentioned you were using reST in the top post, but I completely glanced over it. My comments were relating to markdown input.

I'm not familiar with textlint-plugin-rst (or even that markup), but as this lib deals with the AST only I would hazard a guess that the source of this issue may lie there. Unfortunately I am definitely not the best person to confirm that.

As a co-user of this lib though, it does look to handle things neatly when the input reflects the correct doc semantics.