[PATCH] D39676: [clangd] Add rename support.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 8 02:09:45 PST 2017


ilya-biryukov added a comment.

Just a few minor code style comments.



================
Comment at: clangd/ClangdServer.cpp:367
+    Context.setASTContext(AST->getASTContext());
+    auto rename = clang::tooling::RenameOccurrences::initiate(
+        Context, SourceRange(SourceLocationBeg), NewName.str());
----------------
NIT: local vars are `UpperCamelCase`


================
Comment at: clangd/ClangdUnit.cpp:1399
+
+SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos,
+                                        const FileEntry *FE) {
----------------
sammccall wrote:
> nit: the rest of this file defines functions outside their namespace.
> 
> TBH I prefer the style you're using here, but we should be consistent within a file.
+1. Let's document and enforce the preferred style in new commits.
But better done in a separate commit for all of clangd and let's stick to the original style in this file for now.


https://reviews.llvm.org/D39676





More information about the cfe-commits mailing list