ioeric added inline comments.
================ Comment at: include/clang/Tooling/Refactoring/EditList.h:41 + /// \brief Creates an edit list for a key position. + EditList(const SourceManager &SM, SourceLocation KeyPosition); + ---------------- ioeric wrote: > klimek wrote: > > ioeric wrote: > > > djasper wrote: > > > > I wonder whether we should always use a SourceLocation as key or > > > > whether we want to leave this up to the users. E.g. we could make this > > > > take a string parameter and provide a > > > > > > > > string getKeyForLocation(const SourceManager &SM, SourceLocation > > > > KeyPosition); > > > I think the key idea of EditList is that it groups a set of edits around > > > a key position. For refactoring, using position as key makes sense to me > > > (since all edits will be around some position), and I couldn't think of > > > other things that are good to be a key here. > > An idea would be an edit that just inserts multiple headers, but does not > > directly touch a source location (thus, we might want to key things off of > > files?). > We could make the key position the start of the file in this case. But looks > like a customize-able key could potentially enable more opportunities, I'll > go with Daniel's suggestion. Thanks! So I tried the `getKeyForLocation` way, but we still need `FilePath` in the constructor to fully initialize an EditList. I think it might be better if we keep the current constructor (since this will be the most-commonly used one IMO) and adds another constructor that takes `FilePath` and `Key`. https://reviews.llvm.org/D27054 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
