nridge added a comment. I've addressed some of the review comments, with a view to getting something minimal we can land, and improve on in follow-up changes.
Mostly, I focused on the suggestions which reduce the number of results. I've left other suggestions which increase the number of results (e.g. handling non-indexed symbols) for follow-ups. In D72874#1831977 <https://reviews.llvm.org/D72874#1831977>, @sammccall wrote: > - only trigger when there's *some* positive signal for the word. > - Markup like quotes/backticks/brackets/`\p` > - weird case like `lowerCamel`, `UpperCamel`, `CAPS`, `mid-sentence > Capitalization`, `under_scores`. > - use of the word as a token in nearby code (very close if very short, > anywhere in file if longer) > - (maybe you want to support `ns::Qualifiers`?) I currently handle `lowerCamel`, `UpperCamel`, `CAPS`, and `under_scores`. I've left the others as follow-ups. > - post-filter aggressively - only return exact name matches (I think > including case). Done. > - call `fuzzyFind` directly and set `ProximityPath` Done. > as well as the enclosing scopes from lexing. For extra strictness consider > AnyScope=false I haven't done this yet, do you think it's important for an initial landing? If so, could you mention what API you had in mind for determining "enclosing scopes from lexing"? I had in mind using something like `SelectionTree` and collecting any `RecordDecl`s or `NamespaceDecl`s on the path from the common ancestor to the TU, but that's technically not "from lexing", so perhaps you have something else in mind. > - if you get more than 3 results, and none from current file, maybe don't > return anything, as confidence is too low. Or try a stricter query... I implemented this, but my testing shows this causes a lot of results for class names to be excluded. The reason appears to be that `fuzzyFind()` returns the class and each of its constructors as distinct results, so if a class has more than two constructors, we'll have more than 3 results (and typically the class is declared in a different file). Should we try to handle this case specifically (collapse a class name and its construtors to a single result), or should we reconsider this filtering criterion? It's not exactly clear to me what sort of bad behaviour it's intended to weed out. > - handle the most common case of non-indexable symbols (local symbols) by > running the query against the closest occurrence of the token in code. I've left this as a follow-up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72874/new/ https://reviews.llvm.org/D72874 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
