[PATCH] D61373: Fix OptTable::findNearest() adding delimiter for free

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Thanks for the fast review!



================
Comment at: llvm/lib/Option/OptTable.cpp:294
 
+    std::string NormalizedName = LHS;
+    if (Option.find(Delimiter) == LHS.size())
----------------
MaskRay wrote:
> How about:
> 
> ```
>     StringRef LHS, RHS;
>     char Last = CandidateName.back();
>     std::string NormalizedName = Option;
>     if (Last == '=' || Last == ':') {
>       std::tie(LHS, RHS) = Option.split(Last);
>       NormalizedName = LHS;
>       if (Option.find(Last) == LHS.size())
>         NormalizedName += Last;
>     }
> ```
I like it.

But in a follow-up, I wanted to add the following the the loop below:

```
      if (RHS.empty() && !Delimiter.empty()) {
        // The Candidate ends with a = or : delimiter, but the option passed in
        // didn't contain the delimiter (or doesn't have anything after it).
        // In that case, penalize the correction: `-nodefaultlibs` is more
        // likely to be a spello for `-nodefaultlib` than `-nodefaultlib:`
        // since the latter would need an argument, even though both have
        // an unmodified editing distance of 1.
        Distance++;
      }
```

With `Delimiter` gone, that would become `if (RHS.empty() && (Last == '=' || Last == ':')` -- so I added a `CandidateHasDelimiter` variable too.


================
Comment at: llvm/unittests/Option/OptionParsingTest.cpp:301
 
+  // `-glormp` should have an editing distance of 1 to `-glormp=`.
+  EXPECT_EQ(1U, T.findNearest("--glormp", Nearest));
----------------
MaskRay wrote:
> // `--glormp` should have an editing distance of 1 to `--glormp=`.
Whoops! Thanks.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61373/new/

https://reviews.llvm.org/D61373