[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