textlint icon indicating copy to clipboard operation
textlint copied to clipboard

Feature Request: Multiple fix results and interactive fix

Open io-monad opened this issue 8 years ago • 7 comments

Hi,

I'm just wondering if it is possible to have "Multiple fix results" and "Interactive fix" features in textlint.

For example, "Spell checker" sometimes has multiple candidates of fix result. (ex. buring => burying, burning, burin, during)

The current textlint fixer cannot express this case and gives up on fixing a text. But I think it is "mottainai" to throw away those results.

How about allowing a fixer to return an Object of fixers with expressive titles as keys like following:

report(node, new RuleError(message, {
  index: index,
  fix: {
    "burying": fixer.replaceTextRange(range, "burying"),
    "burning": fixer.replaceTextRange(range, "burning"),
    "burin": fixer.replaceTextRange(range, "burin"),
    "during": fixer.replaceTextRange(range, "during"),
  }
}));

And it is interesting if textlint asks the user which fix result to be applied.

$ textlint --fix test.txt
> Found 4 fix candidates. Choose which one to apply:
  [*] buring
  [ ] burning
  [ ] burin
  [ ] during

... diff result around the chosen fix ...

However, it can break the backward-compatibility so it is up to you to make --fix incompatible, or introduce a new option like --fix-interactive.

Thank you.

io-monad avatar Mar 15 '16 14:03 io-monad

Interesting. Similar discussion is hold on ESLint https://github.com/eslint/eslint/issues/3134

I'm not sure an interactive prompt makes sense given that some run ESLint on massive codebases, you could send up with thousands of prompts, which seems like one of the rings of hell. @nzakas said -- https://github.com/eslint/eslint/issues/3134#issuecomment-137503924

I agree with this.

"Multiple fix results"

It is better that one message fix one issue, I think. It make simple.

Make one fix per message. --- https://github.com/textlint/textlint/blob/master/docs/rule-fixable.md


"Interactive fix"

It is the point of this discussion. I think that it is not "fix", but it is "suggestion".

I'm instersting in How to represent the "suggestion" as a TextLintMessage? The fixable feature use the TextLintMessage format.

  • https://github.com/textlint/textlint/blob/master/docs/formatter.md#result-of-fixing

We want to separate fixable feature as a module in the best-case scenario.

@azu think that --fix is an alias to  
$ textlint --fix [--formatter nothing-formatter] README.md  
# ===
$ textlint --fix --formatter json README.md | textlint-fixer  
-- https://github.com/textlint/textlint/issues/124

On the other hand, what does the "suggestion" feature use format?

azu avatar Mar 15 '16 14:03 azu

Okay, I understand the concept of textlint "Make one fix per message" and making the core as simple as possible.

I agree with you that fixable feature is ideally not a part of textlint's works, since it is "textlint", not "textfix" nor "textsuggest" (I'm writing this again!). So separating fixable feature makes sense to me.

what does the "suggestion" feature use format?

In my opinion, suggestions should not be wrapped by RuleError because they are not "errors".

How about this one:

// Linting errors are reported same as before
report(node, new RuleError(message, { index, fix }));

// Optional suggestions can be emitted
report(node, new RuleSuggestion(message, { index, suggestions }));

Looks complicated?

io-monad avatar Mar 15 '16 15:03 io-monad

Principle: textlint process texts, and output serialized message data.

It is thought from textlint side.

First, We will need to think the serialized message data format like this.

    // TextLintResult object
    {
        filePath: "./myfile.md",
        messages: [
            // TextLintMessage object
            {
                ruleId: "semi",
                line: 1,
                column: 23,
                message: "Expected a semicolon.",
                fix: {
                   text : ";",
                   range: [23,24]
                },
                // ??? What is best?
                suggestions: []
            }
        ]
    }
  • https://github.com/textlint/textlint/blob/master/docs/formatter.md

In my opinion, suggestions should not be wrapped by RuleError because they are not "errors".

API is secondary thing.

azu avatar Mar 15 '16 15:03 azu

It is thought from other side

:bulb: You can implement textsuggest without changing the textlint maybe.

TextLintMessage interface has data property that is any data.

interface TextLintMessage {
    ruleId: string;
    message: string;
    // optional data
    data?: any;
    // FixCommand
    fix?: TextLintFixCommand;
    // location info
    // Text -> AST TxtNode(0-based columns) -> textlint -> TextLintMessage(**1-based columns**)
    line: number; // start with 1
    column: number;// start with 1
    // severity
    /*
     "info": 0,
     "warning": 1,
     "error": 2
     */
    severity?: number;
}
  1. implement API like RuleSuggestion at first
    • publish the API as a npm module, use it via npm.
    • RuleSuggestion put "suggestion" data to data property
  2. implement textsuggest module that is received json data from textlint.
    • $ textlint -f json | textsuggest # show suggest UI
    • textsuggest use data property

But, need to textlintfix cli that doesn't exist currently :cry:

$ textlint -f json | textsuggest | textlintfix

This is forgettable idea...

azu avatar Mar 15 '16 16:03 azu

Related issues: #124. https://github.com/stylelint/stylelint/issues/330

azu avatar Mar 15 '16 16:03 azu

Ah, I see the point. "format" is so confusing word... :sweat_smile:

But I have no idea what is the best for the message format. It at least should have text and range as well as fix. It might optionally have "suggestion message" (like "Correct spelling to buring", etc.).

Using data property is reasonable. Unofficial textsuggest seems interesting to me, but it might be a bit far from the textlint core to encourage plugin developers to support it.

I've got an idea to implement it as a message formatter for TextFixEngine that interacts with a user using data field. But after I scanned some source code of textlint, I saw message formatter is called synchronous. :pensive:

io-monad avatar Mar 15 '16 16:03 io-monad

I saw message formatter is called synchronous.

TextLintEngine#formatResults just use https://github.com/textlint/textlint-formatter . TextFixEngine has formatter limitation #148

azu avatar Mar 16 '16 02:03 azu

  • https://github.com/textlint/textlint/issues/1122

I've created suggestion API proposal. This issue merge into https://github.com/textlint/textlint/issues/1122

azu avatar Mar 24 '23 00:03 azu