[PATCH] D64912: [Clangd] Changed ExtractVariable to only work on non empty selections

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 18 08:07:38 PDT 2019

sammccall added inline comments.

Comment at: clang-tools-extra/clangd/refactor/Tweak.h:51
+    /// The Length of the selection
+    unsigned SelectionLength;
     /// The AST nodes that were selected.
kadircet wrote:
> SureYeaah wrote:
> > kadircet wrote:
> > > maybe expose a `SourceLocation SelectionEnd` ?
> > Should I also have a SelectionStart? Or just use Cursor?
> I think it makes more sense to have `SelectionBegin` than `Cursor, AFAIK we usually don't have a notion of `Cursor` when provided with a selection.
> It only makes sense for inputs coming from clients like vim, in which you only have a cursor rather then a selection. But I would like to know what @sammccall thinks about it as well.
I'd suggest `unsigned SelectionStart, SelectionEnd` as it more clearly communicates this is main-file only and can be easily used with `Code`.

I'd add a FIXME to remove Cursor as it's redundant (we can replace it with a method)

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list