[PATCH] D69184: [libTooling] Introduce general combinator for fixed-value `MatchConsumer`s.
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 23 05:56:16 PDT 2019
ymandel added a comment.
In D69184#1718427 <https://reviews.llvm.org/D69184#1718427>, @ymandel wrote:
> In D69184#1715256 <https://reviews.llvm.org/D69184#1715256>, @gribozavr wrote:
>
> > What are the use cases for non-text values?
> >
> > I like the name `text` much better... If the name conflict is the issue, I think you could rename it to `toText` -- it would even read more fluently `change(toText("abc"))`, also making it obvious that we are not changing *the text abc*, but we are changing the code to say "abc".
>
>
> Another example use is: https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/RangeSelector.h#L29-L32, although I must admit that I hadn't thought of that before this patch. It's just the natural generalization (it's actually the K combinator form SKI fame). So, I haven't thought about other examples -- the primary motivation was just the naming.
>
> That said, I'm not sure about `toText` because I consider the "to" as implicit in `change`. In fact, it might be a good idea to rename `change` to `changeTo` (WDYT?). What about any of `asText`, `textMsg`, `textMessage`? (I realize the unfortunate pun in the last two, but not sure that's worth caring about).
An alternative, is not to introduce a new combinator at all, and just use `Stencil`s:
change(cat("abc"))
Or, add `operator()` to `StencilPart` and use those directly:
change(text("abc"))
Given that `Stencil` and `Transformer` are bundled in the same library, is there any strong reason for a client to avoid a dependency on `Stencil`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69184/new/
https://reviews.llvm.org/D69184
More information about the cfe-commits
mailing list