[PATCH] D80472: [clangd] Add access specifier information to hover contents

This revision was automatically updated to reflect the committed changes. Closed by commit rG6407aa9d2e0e: [clangd] Add access specifier information to hover contents (authored by danielmartin, committed by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80472: [clangd] Add access specifier information to hover contents

danielmartin updated this revision to Diff 266540. danielmartin added a comment. Rebase and squash Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80472/new/ https://reviews.llvm.org/D80472 Files: clang-tools-extra/clang-doc/Generators.cpp

[PATCH] D80472: [clangd] Add access specifier information to hover contents

kadircet added a comment. thanks a lot, looks like you've only uploaded the diff for your latest changes(e.g. changes to clang-doc are gone). you need to squash them and upload a single diff based at master. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D80472: [clangd] Add access specifier information to hover contents

danielmartin added a comment. Thanks for the review! I don't have commit access so I'd need someone to land this patch for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80472/new/ https://reviews.llvm.org/D80472

[PATCH] D80472: [clangd] Add access specifier information to hover contents

danielmartin updated this revision to Diff 266322. danielmartin marked 2 inline comments as done. danielmartin added a comment. Address feedback Rename getAccess to getAccessSpelling. Replace more parts of the codebase that were using their own version of getAccessSpelling. Use StringRef's

[PATCH] D80472: [clangd] Add access specifier information to hover contents

kadircet accepted this revision. kadircet marked an inline comment as done. kadircet added a comment. This revision is now accepted and ready to land. thanks! LGTM with some minor comments. let me know if you don't have commit access so that i can land this for you. Comment

[PATCH] D80472: [clangd] Add access specifier information to hover contents

danielmartin marked 6 inline comments as done. danielmartin added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:680 +StringRef getAccessString(AccessSpecifier AS) { + switch (AS) { kadircet wrote: > it is annoying to have this function

[PATCH] D80472: [clangd] Add access specifier information to hover contents

danielmartin updated this revision to Diff 265942. danielmartin added a comment. Move clang::getAccess to Specifiers.h and refactor logic in clang-doc to use that function instead of its own. Also changes where "public", "private" etc. is shown in the hover contents. Now it's shown at the

[PATCH] D80472: [clangd] Add access specifier information to hover contents

kadircet added a comment. thanks for taking a look at this, this looks great! mostly nits, but the one in tests is important and annoying (it might require you to update some existing cases) Comment at: clang-tools-extra/clangd/Hover.cpp:680 +StringRef

[PATCH] D80472: [clangd] Add access specifier information to hover contents

danielmartin created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. danielmartin edited the summary of this revision. For https://github.com/clangd/clangd/issues/382 This commit adds access