[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
torshepherd wrote: I'll close this patch and pursue the other things we discussed https://github.com/llvm/llvm-project/pull/79448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
https://github.com/torshepherd closed https://github.com/llvm/llvm-project/pull/79448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
HighCommander4 wrote: I read up on this a bit; https://github.com/Microsoft/vscode/issues/62110 seems particularly relevant. Based on [this comment](https://github.com/Microsoft/vscode/issues/62110#issuecomment-456240329): > [...] we have an `auto fix` action in the editor that fixes individual errors it seems that I did misunderstand the purpose of the "Auto fix" command in vscode; it looks like it's only intended to fix a single a error. The proposal in that thread (which has since been implemented in vscode) does include a way to apply all quick-fixes in a file: it's the `"source.fixAll"` protocol described in https://github.com/clangd/clangd/issues/1446. It doesn't have to be invoked automatically on file-save, it can be invoked manually via a command (which in vscode is called "Fix all", and seems to only be enabled if the server advertises support for `"source.fixAll"`). Conclusion: there's probably not much point proceeding with this patch (the slight behaviour change described in the previous comment doesn't seem very helpful), but instead we should fix https://github.com/clangd/clangd/issues/1446 and then the desired scenario should work using the "Fix all" command (and as an added bonus, there is no need to select the entire file contents, "Fix all" automatically applies to the whole file). https://github.com/llvm/llvm-project/pull/79448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
HighCommander4 wrote: > However, this doesn't seem to change the behavior in vscode: I applied the patch locally, and it does have _an_ effect on the steps from #1536 (running `Auto fix` with code containing multiple diagnostics selected): * before, it would show a message saying no fixes are available * now, it shows a popup listing the multiple quick-fixes (but only letting me apply one at a time) Unfortunately, it's not the effect we were hoping for (automatically applying the multiple fixes at once). It's possible that in my analysis in #830 / #1536 I misunderstood the purpose of the VSCode's "Auto fix" command. This will need more investigation on the VSCode side. https://github.com/llvm/llvm-project/pull/79448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
llvmbot wrote:
@llvm/pr-subscribers-clangd
Author: Tor Shepherd (torshepherd)
Changes
This PR attempts to fix [1536.](https://github.com/clangd/clangd/issues/1536).
See in the unit tests, when all quick fixes are of the same `code`,
`isPreferred` will be true. However, this doesn't seem to change the behavior
in vscode:
---
Full diff: https://github.com/llvm/llvm-project/pull/79448.diff
3 Files Affected:
- (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+41-37)
- (modified)
clang-tools-extra/clangd/test/fixits-codeaction-documentchanges.test (+2)
- (modified) clang-tools-extra/clangd/test/fixits-codeaction.test (+2)
``diff
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9..5fbd09fdcfdf42 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -27,7 +27,9 @@
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Allocator.h"
@@ -117,10 +119,9 @@ CodeAction toCodeAction(const Fix , const URIForFile
,
Edit.textDocument = VersionedTextDocumentIdentifier{{File}, Version};
for (const auto : F.Edits)
Edit.edits.push_back(
- {E.range, E.newText,
- SupportChangeAnnotation ? E.annotationId : ""});
+ {E.range, E.newText, SupportChangeAnnotation ? E.annotationId : ""});
if (SupportChangeAnnotation) {
- for (const auto &[AID, Annotation]: F.Annotations)
+ for (const auto &[AID, Annotation] : F.Annotations)
Action.edit->changeAnnotations[AID] = Annotation;
}
}
@@ -861,24 +862,24 @@ void ClangdLSPServer::onRename(const RenameParams ,
if (!Server->getDraft(File))
return Reply(llvm::make_error(
"onRename called for non-added file", ErrorCode::InvalidParams));
- Server->rename(File, Params.position, Params.newName, Opts.Rename,
- [File, Params, Reply = std::move(Reply),
- this](llvm::Expected R) mutable {
- if (!R)
- return Reply(R.takeError());
- if (auto Err = validateEdits(*Server, R->GlobalChanges))
- return Reply(std::move(Err));
- WorkspaceEdit Result;
- // FIXME: use documentChanges if SupportDocumentChanges is
- // true.
- Result.changes.emplace();
- for (const auto : R->GlobalChanges) {
- (*Result
- .changes)[URI::createFile(Rep.first()).toString()] =
- Rep.second.asTextEdits();
- }
- Reply(Result);
- });
+ Server->rename(
+ File, Params.position, Params.newName, Opts.Rename,
+ [File, Params, Reply = std::move(Reply),
+ this](llvm::Expected R) mutable {
+if (!R)
+ return Reply(R.takeError());
+if (auto Err = validateEdits(*Server, R->GlobalChanges))
+ return Reply(std::move(Err));
+WorkspaceEdit Result;
+// FIXME: use documentChanges if SupportDocumentChanges is
+// true.
+Result.changes.emplace();
+for (const auto : R->GlobalChanges) {
+ (*Result.changes)[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+}
+Reply(Result);
+ });
}
void ClangdLSPServer::onDocumentDidClose(
@@ -1014,7 +1015,7 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams
,
std::map ToLSPDiags;
ClangdServer::CodeActionInputs Inputs;
- for (const auto& LSPDiag : Params.context.diagnostics) {
+ for (const auto : Params.context.diagnostics) {
if (auto DiagRef = getDiagRef(File.file(), LSPDiag)) {
ToLSPDiags[*DiagRef] = LSPDiag;
Inputs.Diagnostics.push_back(*DiagRef);
@@ -1023,13 +1024,9 @@ void ClangdLSPServer::onCodeAction(const
CodeActionParams ,
Inputs.File = File.file();
Inputs.Selection = Params.range;
Inputs.RequestedActionKinds = Params.context.only;
- Inputs.TweakFilter = [this](const Tweak ) {
-return Opts.TweakFilter(T);
- };
- auto CB = [this,
- Reply = std::move(Reply),
- ToLSPDiags = std::move(ToLSPDiags), File,
- Selection = Params.range](
+ Inputs.TweakFilter = [this](const Tweak ) { return Opts.TweakFilter(T); };
+ auto CB = [this, Reply = std::move(Reply), ToLSPDiags =
std::move(ToLSPDiags),
+ File, Selection = Params.range](
llvm::Expected Fixits) mutable
{
if (!Fixits)
return Reply(Fixits.takeError());
@@ -1038,27
[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/79448 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Attempt to fix https://github.com/clangd/clangd/issues/1536 (PR #79448)
https://github.com/torshepherd created
https://github.com/llvm/llvm-project/pull/79448
This PR attempts to fix [1536.](https://github.com/clangd/clangd/issues/1536).
See in the unit tests, when all quick fixes are of the same `code`,
`isPreferred` will be true. However, this doesn't seem to change the behavior
in vscode:
>From 6bb871f64073132683bab2318c228a4ef7723c8e Mon Sep 17 00:00:00 2001
From: Tor Shepherd
Date: Wed, 24 Jan 2024 23:43:52 -0500
Subject: [PATCH 1/2] second try
---
clang-tools-extra/clangd/ClangdLSPServer.cpp | 78 ++--
1 file changed, 41 insertions(+), 37 deletions(-)
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a87da252b7a7e9b..5fbd09fdcfdf420 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -27,7 +27,9 @@
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Allocator.h"
@@ -117,10 +119,9 @@ CodeAction toCodeAction(const Fix , const URIForFile
,
Edit.textDocument = VersionedTextDocumentIdentifier{{File}, Version};
for (const auto : F.Edits)
Edit.edits.push_back(
- {E.range, E.newText,
- SupportChangeAnnotation ? E.annotationId : ""});
+ {E.range, E.newText, SupportChangeAnnotation ? E.annotationId : ""});
if (SupportChangeAnnotation) {
- for (const auto &[AID, Annotation]: F.Annotations)
+ for (const auto &[AID, Annotation] : F.Annotations)
Action.edit->changeAnnotations[AID] = Annotation;
}
}
@@ -861,24 +862,24 @@ void ClangdLSPServer::onRename(const RenameParams ,
if (!Server->getDraft(File))
return Reply(llvm::make_error(
"onRename called for non-added file", ErrorCode::InvalidParams));
- Server->rename(File, Params.position, Params.newName, Opts.Rename,
- [File, Params, Reply = std::move(Reply),
- this](llvm::Expected R) mutable {
- if (!R)
- return Reply(R.takeError());
- if (auto Err = validateEdits(*Server, R->GlobalChanges))
- return Reply(std::move(Err));
- WorkspaceEdit Result;
- // FIXME: use documentChanges if SupportDocumentChanges is
- // true.
- Result.changes.emplace();
- for (const auto : R->GlobalChanges) {
- (*Result
- .changes)[URI::createFile(Rep.first()).toString()] =
- Rep.second.asTextEdits();
- }
- Reply(Result);
- });
+ Server->rename(
+ File, Params.position, Params.newName, Opts.Rename,
+ [File, Params, Reply = std::move(Reply),
+ this](llvm::Expected R) mutable {
+if (!R)
+ return Reply(R.takeError());
+if (auto Err = validateEdits(*Server, R->GlobalChanges))
+ return Reply(std::move(Err));
+WorkspaceEdit Result;
+// FIXME: use documentChanges if SupportDocumentChanges is
+// true.
+Result.changes.emplace();
+for (const auto : R->GlobalChanges) {
+ (*Result.changes)[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+}
+Reply(Result);
+ });
}
void ClangdLSPServer::onDocumentDidClose(
@@ -1014,7 +1015,7 @@ void ClangdLSPServer::onCodeAction(const CodeActionParams
,
std::map ToLSPDiags;
ClangdServer::CodeActionInputs Inputs;
- for (const auto& LSPDiag : Params.context.diagnostics) {
+ for (const auto : Params.context.diagnostics) {
if (auto DiagRef = getDiagRef(File.file(), LSPDiag)) {
ToLSPDiags[*DiagRef] = LSPDiag;
Inputs.Diagnostics.push_back(*DiagRef);
@@ -1023,13 +1024,9 @@ void ClangdLSPServer::onCodeAction(const
CodeActionParams ,
Inputs.File = File.file();
Inputs.Selection = Params.range;
Inputs.RequestedActionKinds = Params.context.only;
- Inputs.TweakFilter = [this](const Tweak ) {
-return Opts.TweakFilter(T);
- };
- auto CB = [this,
- Reply = std::move(Reply),
- ToLSPDiags = std::move(ToLSPDiags), File,
- Selection = Params.range](
+ Inputs.TweakFilter = [this](const Tweak ) { return Opts.TweakFilter(T); };
+ auto CB = [this, Reply = std::move(Reply), ToLSPDiags =
std::move(ToLSPDiags),
+ File, Selection = Params.range](
llvm::Expected Fixits) mutable
{
if (!Fixits)
return Reply(Fixits.takeError());
@@ -1038,27 +1035,34 @@ void
