sammccall added a comment. This is cool! Detecting basic typos could save a fair bit of time. I do want to be a bit wary of spending too much complexity on small enhancements to rarely used code - if it's more than a simple typo we're correcting, then I think it's ok to make the user look it up.
In D92990#2445283 <https://reviews.llvm.org/D92990#2445283>, @njames93 wrote: > Adding in support for fix-its(at least in vscode) didn't seem to work. The > language server there doesn't seem to send the `textDocument/codeAction` > message to the server for the `.clangd` file. Right, we can publish to any file, but we only expect to get requests for files we're active on, and users will expect us to track draft changes etc. I don't think we want to go down the path of being a full language server for config files, so I wouldn't pursue fixit support. ================ Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:202 if (Warn) - Outer->warning("Unknown " + Description + " key " + **Key, *K); + UnknownKeys.push_back(std::move(*Key)); } ---------------- i'm not sure it's actually worth the complexity to delay this, track the seen keys, and exclude them from the analysis. Offering a suggestion already seems pretty good, but I don't think we should try too hard to make it perfect. ================ Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:211 + llvm::SmallVector<llvm::StringRef, 4> + getUnseenKeys(const llvm::SmallSet<std::string, 8> &SeenKeys) const { + llvm::SmallVector<llvm::StringRef, 4> Result; ---------------- nit: drop explicit '4' size here also drop `get` prefix here and for best-guess (matching the local style for functions used for value rather than for side-effect) ================ Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:221 + getBestGuess(llvm::StringRef Search, llvm::ArrayRef<llvm::StringRef> Items, + unsigned MaxEdit = 0) { + ---------------- nit: drop unused default param ================ Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:223 + + // If Search is sufficiently short (size() < 2), just return nothing. + if (Search.size() < 2) ---------------- Comment here just echoes the code - remove? ================ Comment at: clang-tools-extra/clangd/ConfigYAML.cpp:231 + unsigned EditDistance = Search.edit_distance(Item, true, MaxEdit); + if (EditDistance == MaxEdit) { + if (!Result) ---------------- This branch is confusing. It seems to be saying if we have a tie for best, don't return either result. Why is that important/worth any complexity? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92990/new/ https://reviews.llvm.org/D92990 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
