[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2022-04-07 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 421371. njames93 added a comment. Use new tweak InsertionPoint logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942 Files:

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2022-03-31 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D94942#3419616 , @sammccall wrote: > @njames93 I had completely forgotten about this when I attempted the same > thing early this year. > I never finished it but wanted to share what I had in case it's useful: >

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2022-03-31 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Herald added a project: All. @njames93 I had completely forgotten about this when I attempted the same thing early this year. I never finished it but wanted to share what I had in case it's useful: http://reviews.llvm.org/D122827. Also happy to finish that sometime if

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-04-22 Thread Christopher Rhodes via Phabricator via cfe-commits
crr0004 added a comment. One thing I noticed was that you need to be over the base class or overriding class identifier in order to get the code action. For some editors like Visual Studio Code this is less noticeable but for others like VIM it is more noticeable as you need to know it's

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-04-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ImplementAbstract.cpp:1 +//===--- ImplementAbstract.cpp ---*- C++-*-===// +// sammccall wrote: > I'd consider calling this OverrideVirtual.cpp.

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-04-14 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Herald added a project: clang-tools-extra. Thanks for working on this and sorry for the long delay! I also think it's really useful, I hope we can simplify the impl a little and land it. Comment at:

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-03-10 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329680. njames93 added a comment. Tweak the prepare method to just check for methods that need implementations, this saves some work that could be deferred to the apply method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-28 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 319898. njames93 added a comment. - Fix an issue where replacements could conflict with each other. - Add support for defining method stubs instead of just declaring them, possibly need a way to configure this behaviour. - Change tests to ignore whitespace

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 318932. njames93 added a comment. Update getSelectedRecord to work now that BaseSpecifier may contain nested Nodes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-22 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 318678. njames93 edited the summary of this revision. njames93 added a comment. Fix failing tests. Updated message for tweak from a specified base class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-22 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ImplementAbstract.cpp:354 + return "Implement pure virtual methods from '" + + FromBase->getType().getAsString() + "'"; +} Maybe this should be changed as

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-22 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 318552. njames93 added a comment. If cursor is over one of the base specifiers, offer to implement only the pure methods from that base class. Depends on D95231 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-22 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. In D94942#2515049 , @kadircet wrote: > Thanks this looks great, and something i've been longing for some time! But I > got a couple of questions about the how the interaction is designed (sorry if > I missed some high level

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Thanks this looks great, and something i've been longing for some time! But I got a couple of questions about the how the interaction is designed (sorry if I missed some high level discussions elsewhere, feel free to just point me in that direction). - Why do we just

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-21 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang-tools-extra/clangd/refactor/tweaks/ImplementAbstract.cpp:38-40 + // If we have any pure virtual methods declared in the root (The class + // this tweak was invoked on), assume the user probably doesn't want to + //

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-21 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 318315. njames93 added a comment. Split up the code a little more. Fix a few malformed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942 Files:

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-20 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 317984. njames93 added a comment. - Replace getFinalOverrides for a manual implementation, that method wasn't quite suited to what was needed w.r.t tracking access. - Add support for template classes with no dependant bases. - Add tests for template

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-19 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 317523. njames93 added a comment. Update printing policy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942 Files:

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 317456. njames93 added a comment. Forgot to add the untracked files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94942/new/ https://reviews.llvm.org/D94942 Files:

[PATCH] D94942: [clangd] Add tweak for implementing abstract class

2021-01-18 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: sammccall, kadircet. Herald added subscribers: usaxena95, arphaman, mgorny. njames93 requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Add a tweak that will