[PATCH] D66343: [clangd] Simplify code of ClangdLSPServer::onCommand

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369100: [clangd] Simplify code of ClangdLSPServer::onCommand 
(authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66343?vs=215571=215573#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66343/new/

https://reviews.llvm.org/D66343

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp

Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -570,28 +570,28 @@
 
 void ClangdLSPServer::onCommand(const ExecuteCommandParams ,
 Callback Reply) {
-  auto ApplyEdit = [this](WorkspaceEdit WE,
-  Callback Reply) {
+  auto ApplyEdit = [this](WorkspaceEdit WE, std::string SuccessMessage,
+  decltype(Reply) Reply) {
 ApplyWorkspaceEditParams Edit;
 Edit.edit = std::move(WE);
-call("workspace/applyEdit", std::move(Edit), std::move(Reply));
+call(
+"workspace/applyEdit", std::move(Edit),
+[Reply = std::move(Reply), SuccessMessage = std::move(SuccessMessage)](
+llvm::Expected Response) mutable {
+  if (!Response)
+return Reply(Response.takeError());
+  if (!Response->applied) {
+std::string Reason = Response->failureReason
+ ? *Response->failureReason
+ : "unknown reason";
+return Reply(llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+("edits were not applied: " + Reason).c_str()));
+  }
+  return Reply(SuccessMessage);
+});
   };
-  // FIXME: this lambda is tangled and confusing, refactor it.
-  auto ReplyAfterApplyingEdit =
-  [](decltype(Reply) Reply, std::string SuccessMessage,
- llvm::Expected Response) {
-if (!Response)
-  return Reply(Response.takeError());
-if (!Response->applied) {
-  std::string Reason = Response->failureReason
-   ? *Response->failureReason
-   : "unknown reason";
-  return Reply(llvm::createStringError(
-  llvm::inconvertibleErrorCode(),
-  ("edits were not applied: " + Reason).c_str()));
-}
-return Reply(SuccessMessage);
-  };
+
   if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
   Params.workspaceEdit) {
 // The flow for "apply-fix" :
@@ -602,12 +602,7 @@
 // 5. We unwrap the changes and send them back to the editor
 // 6. The editor applies the changes (applyEdit), and sends us a reply
 // 7. We unwrap the reply and send a reply to the editor.
-ApplyEdit(*Params.workspaceEdit,
-  [Reply = std::move(Reply), ReplyAfterApplyingEdit](
-  llvm::Expected Response) mutable {
-ReplyAfterApplyingEdit(std::move(Reply), "Fix applied.",
-   std::move(Response));
-  });
+ApplyEdit(*Params.workspaceEdit, "Fix applied.", std::move(Reply));
   } else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK &&
  Params.tweakArgs) {
 auto Code = DraftMgr.getDraft(Params.tweakArgs->file.file());
@@ -616,32 +611,29 @@
   llvm::inconvertibleErrorCode(),
   "trying to apply a code action for a non-added file"));
 
-auto Action = [this, ApplyEdit, ReplyAfterApplyingEdit,
-   Reply = std::move(Reply), File = Params.tweakArgs->file,
-   Code = std::move(*Code)](
+auto Action = [this, ApplyEdit, Reply = std::move(Reply),
+   File = Params.tweakArgs->file, Code = std::move(*Code)](
   llvm::Expected R) mutable {
   if (!R)
 return Reply(R.takeError());
 
-  if (R->ApplyEdit) {
-WorkspaceEdit WE;
-WE.changes.emplace();
-(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
-ApplyEdit(
-std::move(WE),
-[Reply = std::move(Reply), ReplyAfterApplyingEdit](
-llvm::Expected Response) mutable {
-  ReplyAfterApplyingEdit(std::move(Reply), "Tweak applied.",
- std::move(Response));
-});
-  }
+  assert(R->ShowMessage || R->ApplyEdit && "tweak has no effect");
+
   if (R->ShowMessage) {
 ShowMessageParams Msg;
 Msg.message = *R->ShowMessage;
 Msg.type = MessageType::Info;
 notify("window/showMessage", Msg);
-Reply("Tweak applied.");
   }
+  if (R->ApplyEdit) {
+

[PATCH] D66343: [clangd] Simplify code of ClangdLSPServer::onCommand

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66343/new/

https://reviews.llvm.org/D66343



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66343: [clangd] Simplify code of ClangdLSPServer::onCommand

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: hokein.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

By inlining a complicated lambda into its single call-site.

Also ensure we call Reply() exactly once even if tweaks return both
ShowMessage and ApplyEdit effects.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66343

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp

Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -570,28 +570,28 @@
 
 void ClangdLSPServer::onCommand(const ExecuteCommandParams ,
 Callback Reply) {
-  auto ApplyEdit = [this](WorkspaceEdit WE,
-  Callback Reply) {
+  auto ApplyEdit = [this](WorkspaceEdit WE, std::string SuccessMessage,
+  decltype(Reply) Reply) {
 ApplyWorkspaceEditParams Edit;
 Edit.edit = std::move(WE);
-call("workspace/applyEdit", std::move(Edit), std::move(Reply));
+call(
+"workspace/applyEdit", std::move(Edit),
+[Reply = std::move(Reply), SuccessMessage = std::move(SuccessMessage)](
+llvm::Expected Response) mutable {
+  if (!Response)
+return Reply(Response.takeError());
+  if (!Response->applied) {
+std::string Reason = Response->failureReason
+ ? *Response->failureReason
+ : "unknown reason";
+return Reply(llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+("edits were not applied: " + Reason).c_str()));
+  }
+  return Reply(SuccessMessage);
+});
   };
-  // FIXME: this lambda is tangled and confusing, refactor it.
-  auto ReplyAfterApplyingEdit =
-  [](decltype(Reply) Reply, std::string SuccessMessage,
- llvm::Expected Response) {
-if (!Response)
-  return Reply(Response.takeError());
-if (!Response->applied) {
-  std::string Reason = Response->failureReason
-   ? *Response->failureReason
-   : "unknown reason";
-  return Reply(llvm::createStringError(
-  llvm::inconvertibleErrorCode(),
-  ("edits were not applied: " + Reason).c_str()));
-}
-return Reply(SuccessMessage);
-  };
+
   if (Params.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND &&
   Params.workspaceEdit) {
 // The flow for "apply-fix" :
@@ -602,12 +602,7 @@
 // 5. We unwrap the changes and send them back to the editor
 // 6. The editor applies the changes (applyEdit), and sends us a reply
 // 7. We unwrap the reply and send a reply to the editor.
-ApplyEdit(*Params.workspaceEdit,
-  [Reply = std::move(Reply), ReplyAfterApplyingEdit](
-  llvm::Expected Response) mutable {
-ReplyAfterApplyingEdit(std::move(Reply), "Fix applied.",
-   std::move(Response));
-  });
+ApplyEdit(*Params.workspaceEdit, "Fix applied.", std::move(Reply));
   } else if (Params.command == ExecuteCommandParams::CLANGD_APPLY_TWEAK &&
  Params.tweakArgs) {
 auto Code = DraftMgr.getDraft(Params.tweakArgs->file.file());
@@ -616,32 +611,29 @@
   llvm::inconvertibleErrorCode(),
   "trying to apply a code action for a non-added file"));
 
-auto Action = [this, ApplyEdit, ReplyAfterApplyingEdit,
-   Reply = std::move(Reply), File = Params.tweakArgs->file,
-   Code = std::move(*Code)](
+auto Action = [this, ApplyEdit, Reply = std::move(Reply),
+   File = Params.tweakArgs->file, Code = std::move(*Code)](
   llvm::Expected R) mutable {
   if (!R)
 return Reply(R.takeError());
 
-  if (R->ApplyEdit) {
-WorkspaceEdit WE;
-WE.changes.emplace();
-(*WE.changes)[File.uri()] = replacementsToEdits(Code, *R->ApplyEdit);
-ApplyEdit(
-std::move(WE),
-[Reply = std::move(Reply), ReplyAfterApplyingEdit](
-llvm::Expected Response) mutable {
-  ReplyAfterApplyingEdit(std::move(Reply), "Tweak applied.",
- std::move(Response));
-});
-  }
+  assert(R->ShowMessage || R->ApplyEdit && "tweak has no effect");
+
   if (R->ShowMessage) {
 ShowMessageParams Msg;
 Msg.message = *R->ShowMessage;
 Msg.type = MessageType::Info;
 notify("window/showMessage", Msg);
-Reply("Tweak applied.");
   }
+  if (R->ApplyEdit) {
+WorkspaceEdit WE;
+WE.changes.emplace();
+(*WE.changes)[File.uri()] =