[PATCH] D66637: [clangd] Support multifile edits as output of Tweaks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:130
+/// pointed by FID.
+Tweak::Effect fileEdit(const SourceManager &SM, FileID FID,
+                       tooling::Replacements Replacements);
----------------
kadircet wrote:
> kadircet wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > what will this be used for?
> > > > 
> > > > You can use at most one of these Effect factories (unless we're going to do some convoluted merge thing), so this seems useful if you've got exactly one edit to a non-main file.
> > > > 
> > > > It seems more useful to return a pair<string, Edit> which could be inserted into a map
> > > why are these moved out of the Effect class?
> > > need to provide a fully-qualified name (I don't think the implementations in this patch actually do that)
> > 
> > I've thought you were also requesting these to be helper functions rather than static methods so that callers don't need to qualify the call.
> > Can move it back into class if I misunderstood your point, but this actually seems a lot cleaner.
> > what will this be used for?
> 
> you are right it doesn't really compose well, changed it to return the pair instead.
moved back into class


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66637