sam-mccall wrote:
> For some context: When I’m talking about * finding the ranges to rename based
> on an index that’s not clangd’s built-in index* I meant a request like
> [apple#7973](https://github.com/apple/llvm-project/pull/7973).
I see. That makes sense, it's just a bit non-obvious because we don't usually
design these pieces as libraries to be consumed outside clangd (either by code
calling into clangd's internals, or a modified version of clangd).
I think we usually won't support major design changes to accommodate these, but
small ones like this look totally fine.
(There are exceptions, e.g. adding XPC support required significant changes.
After these, XPC lives in llvm-project but could just as easily be downstream).
> Functionality-wise, I would be fine with using a `optional<vector<string>>`
> instead of `Selector` in `collectRenameIdentifierRanges`. FWIW I disagree
> that using a `vector<string>` is cleaner than using a dedicated type for it
> because there’s no type-level information about what the `vector<string>`
> represents
Yeah, "clean" can mean various things, e.g. "clearly communicating intent" here
is in tension with "fewer moving parts".
I'd be fine with `struct SelectorName { vector<StringRef> Chunks; }` or `using
SelectorName = vector<string>`, more than that is (for my taste) more noise
than signal here. I think it should go in `clangd/refactor/Rename.h` though -
it's just a part of that API.
https://github.com/llvm/llvm-project/pull/82061
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
