textlint / textlint-plugin-latex2e

Textlint Plugin LaTeX2ε
MIT License
68 stars 8 forks source link

Comments in the equeations #60

Closed tani closed 3 years ago

tani commented 3 years ago

I don't know well but he say so. I should check it later. https://twitter.com/coriandered/status/1350066641451642882?s=20

If not, I close this issue.

pddg commented 3 years ago

Sample TeX source is as follows.

\documentclass[12pt]{jarticle}

\begin{document}

\begin{equation}
    % comment inside equation
    x = x^2
\end{equation}

\end{document}
Parsed results by `latex-utensils` ```js { kind: 'ast.root', content: [ { kind: 'command', name: 'documentclass', args: [ { kind: 'arg.optional', content: [ { kind: 'text.string', content: '12pt', location: { start: { offset: 15, line: 1, column: 16 }, end: { offset: 19, line: 1, column: 20 } } } ], location: { start: { offset: 14, line: 1, column: 15 }, end: { offset: 20, line: 1, column: 21 } } }, { kind: 'arg.group', content: [ { kind: 'text.string', content: 'jarticle', location: { start: { offset: 21, line: 1, column: 22 }, end: { offset: 29, line: 1, column: 30 } } } ], location: { start: { offset: 20, line: 1, column: 21 }, end: { offset: 30, line: 1, column: 31 } } } ], location: { start: { offset: 0, line: 1, column: 1 }, end: { offset: 30, line: 1, column: 31 } } }, { kind: 'parbreak', location: { start: { offset: 30, line: 1, column: 31 }, end: { offset: 32, line: 3, column: 1 } } }, { kind: 'env', name: 'document', args: [], content: [ { kind: 'parbreak', location: { start: { offset: 48, line: 3, column: 17 }, end: { offset: 50, line: 5, column: 1 } } }, { kind: 'env.math.align', name: 'equation', args: [], content: [ { kind: 'math.character', content: 'x' }, { kind: 'math.character', content: '=' }, { kind: 'math.character', content: 'x' }, { kind: 'superscript', arg: { kind: 'math.character', content: '2' }, location: { start: { offset: 106, line: 7, column: 10 }, end: { offset: 109, line: 8, column: 1 } } } ], location: { start: { offset: 50, line: 5, column: 1 }, end: { offset: 123, line: 8, column: 15 } } }, { kind: 'parbreak', location: { start: { offset: 123, line: 8, column: 15 }, end: { offset: 125, line: 10, column: 1 } } } ], location: { start: { offset: 32, line: 3, column: 1 }, end: { offset: 139, line: 10, column: 15 } } }, { kind: 'parbreak', location: { start: { offset: 139, line: 10, column: 15 }, end: { offset: 140, line: 11, column: 1 } } } ], comment: [ { kind: 'comment', content: ' comment inside equation', location: { start: { offset: 71, line: 6, column: 5 }, end: { offset: 97, line: 7, column: 1 } } } ] } ```

Each math character in equation environment does not have location information. So the completeComment function will fail to insert a comment, I think.

pddg commented 3 years ago

The problem is not fixed in the latest version of latex-utensils (v4.0.0-beta.5). We may need a workaround.

pddg commented 3 years ago

Further investigation is here.

\documentclass[12pt]{jarticle}

\begin{document}

\begin{equation}
    % comment1
    x = y % comment2
    x = y
    % comment3
\end{equation}

\end{document}
Parsed results by `latex-utensils` ```js { kind: 'ast.root', content: [ { kind: 'command', name: 'documentclass', args: [ { kind: 'arg.optional', content: [ { kind: 'text.string', content: '12pt', location: { start: { offset: 15, line: 1, column: 16 }, end: { offset: 19, line: 1, column: 20 } } } ], location: { start: { offset: 14, line: 1, column: 15 }, end: { offset: 20, line: 1, column: 21 } } }, { kind: 'arg.group', content: [ { kind: 'text.string', content: 'jarticle', location: { start: { offset: 21, line: 1, column: 22 }, end: { offset: 29, line: 1, column: 30 } } } ], location: { start: { offset: 20, line: 1, column: 21 }, end: { offset: 30, line: 1, column: 31 } } } ], location: { start: { offset: 0, line: 1, column: 1 }, end: { offset: 30, line: 1, column: 31 } } }, { kind: 'parbreak', location: { start: { offset: 30, line: 1, column: 31 }, end: { offset: 32, line: 3, column: 1 } } }, { kind: 'env', name: 'document', args: [], content: [ { kind: 'parbreak', location: { start: { offset: 48, line: 3, column: 17 }, end: { offset: 50, line: 5, column: 1 } } }, { kind: 'env.math.align', name: 'equation', args: [], content: [ { kind: 'math.character', content: 'x' }, { kind: 'math.character', content: '=' }, { kind: 'math.character', content: 'y' }, { kind: 'math.character', content: 'x' }, { kind: 'math.character', content: '=' }, { kind: 'math.character', content: 'y' } ], location: { start: { offset: 50, line: 5, column: 1 }, end: { offset: 142, line: 10, column: 15 } } }, { kind: 'parbreak', location: { start: { offset: 142, line: 10, column: 15 }, end: { offset: 144, line: 12, column: 1 } } } ], location: { start: { offset: 32, line: 3, column: 1 }, end: { offset: 158, line: 12, column: 15 } } } ], comment: [ { kind: 'comment', content: ' comment1', location: { start: { offset: 71, line: 6, column: 5 }, end: { offset: 82, line: 7, column: 1 } } }, { kind: 'comment', content: ' comment2', location: { start: { offset: 92, line: 7, column: 11 }, end: { offset: 103, line: 8, column: 1 } } }, { kind: 'comment', content: ' comment3', location: { start: { offset: 117, line: 9, column: 5 }, end: { offset: 128, line: 10, column: 1 } } } ] } ```

From this result, we found that we could not determine where these comments were located in the equation environment. I think it's better to just give a warning and ignore it than to insert a node in an uncertain position.

pddg commented 3 years ago

@azu cc: @tani Is there a correct way to show some warnings from a plugin? We want to show a warning about the parsing of the text due to the limitation of the parser.

azu commented 3 years ago

There is no way from the plugin. We can not output to stdout by default, because Some formatter like -f json requires parsable output. If you want to output a warning into stdout, we need to detect the current formatted. (However, I know that some users use -f unix as parsable format…)

In textlint core, just throw an error or use debug package. In textlint rule, just use report().

We may want to get a feature that we can add warning as error of rule from core/plugin.

pddg commented 3 years ago

It may be a good idea to use the debug package to let the user choose whether to output or not. From the looks of it, the debug package appears to output logs to stderr, so it shouldn't interfere with software that uses the output of stdout. However, I thought it would be safer to disable the output by default in case there is a problem.

When a user reports unintended behavior, we want to know if it is related to this issue, and we need an easy way to find out. For this reason, I thought it would not be necessary to display it all the time.

tani commented 3 years ago

@pddg I'd like to hear wether textlint-rule-latex-utensils is possible or not. For example, exlint-plugin-prettier and eslint-config-prettier resolve the incompatibles among prettier and eslint. As the same way, can we write such a rule?

pddg commented 3 years ago

@tani What does it resolve? For example, eslint-config-prettier was responsible for turning off all eslint configurations to prevent conflicts between eslint and prettier. And eslint-plugin-prettier is a plugin that runs prettier instead of eslint when it runs, and reports detected errors as eslint errors.

If that is the rule for detecting the problem discussed in this issue, then I think it is difficult. This is because a rule for textlint can only handle the correct AST of textlint. In this case, it will not be able to detect the violation because this plugin cannot be built as a correct AST.

tani commented 3 years ago

Yes, you're right. We run Textlint only for valid AST. My idea is that the plugin inserts dummy node in that place and the rule raises at the node. If the user enable the rule, users can know the error. Otherwise, other rules skip the node.

Perhaps, is it difficult to insert dummy node?

pddg commented 3 years ago

I think it depends on how much "correctness" is required in the ASTs that are generated. I explained above that the math.character inside the equation environment doesn't have any location information, so it can't identify the correct location of comments that should be around it.

So there are two possible ways to insert a dummy node.

  1. Insertion at the most probable location
  2. Insertion in a specific location independent of the generated AST

The first method is to use the fact that the equation environment has location information and insert a comment at the beginning or end of its children. This is easy to do, but it will result in insertion in a location completely different from the original one. In addition, with the expressive power of textlint's AST, it would be difficult to distinguish this from a normal Comment, so the AST may need to be extended.

The second method is difficult to implement. Currently, completeComment is implemented as a recursive function, so it is not easy to escape in the middle and insert a specific node into another part of the body. I will add the comment nodes that could not be inserted to the return value of the insertComment function.

pddg commented 3 years ago

I remembered the textlint --fix. Wherever we insert a dummy comment, the comments in the equation environment are lost because the original position cannot be restored.

pddg commented 3 years ago

enableMathCharacterLocation option is added since v3.0.0. I will update latex-utensils and test it!

pddg commented 3 years ago

@tani I was a little misunderstood. The fact that math.character has no location information was of course a major factor, but before that, we had interpreted env.math.align (and also other math environments) as CodeBlock. CodeBlock has no child nodes.

We have two options.

  1. Parse equation environment as CodeBlock and ignore the comment nodes in it.
  2. Parse equation environment as CodeBlock but add children to it.

Do you have any plans?

tani commented 3 years ago

@tani I was a little misunderstood. The fact that math.character has no location information was of course a major factor, but before that, we had interpreted env.math.align (and also other math environments) as CodeBlock. CodeBlock has no child nodes.

Thank you for working on this issue. Yes, I used to assign the node as CodeBlock since the initial release.

We have two options.

  1. Parse equation environment as CodeBlock and ignore the comment nodes in it.
  2. Parse equation environment as CodeBlock but add children to it.

The refenrence parser plugin of textlint also has Markdown's CodeBlock, which has no children. I feel it is the best option to follow the reference plugin.

tani commented 3 years ago

If someone has a Twitter account, tell him this issue please.

pddg commented 3 years ago

done https://twitter.com/pudding_info/status/1355481423264706564?s=20

tani commented 3 years ago

Thank you!