[PATCH] D39430: [clangd] formatting: don't ignore style
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 2 10:35:05 PDT 2017
sammccall added a comment.
Thanks for this!
================
Comment at: clangd/ClangdLSPServer.cpp:93
+void ClangdLSPServer::replyWithTextEditsOrError(
+ Ctx C, std::string Code,
----------------
This function is pretty hard to understand from a high level - it's a bit of an odd split.
Is the duplication really so bad? The straight-line code could be easy to read:
auto File = Params.textDocument.uri.file;
auto Edits = Server.formatRange(File, Params.range);
if (Edits)
C.reply(replacementsToEdits(Server.getDocument(File), Edits.get()));
else
C.replyError(Edits.takeError());
(The "[" won't be needed after D39435, and we should add a replyError() overload that takes an Error)
================
Comment at: clangd/ClangdLSPServer.cpp:110
+ Server.formatOnType(File, Params.position),
+ "Could not find .clang-format file!");
}
----------------
It doesn't seem reasonable to assume the error is always `.clang-format`. Can we just include the message in the error object?
================
Comment at: clangd/ClangdServer.cpp:41
// Call clang-format.
- // FIXME: Don't ignore style.
- format::FormatStyle Style = format::getLLVMStyle();
- auto Result = format::reformat(Style, Code, Ranges, Filename);
-
+ auto StyleOrError = format::getStyle("file", Filename, "LLVM");
+ if (!StyleOrError)
----------------
This needs to use the VFS I think. (I'm not familiar with the details of the VFS usage I'm afraid)
https://reviews.llvm.org/D39430
More information about the cfe-commits
mailing list