https://github.com/tom-anders closed
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/HighCommander4 updated
https://github.com/llvm/llvm-project/pull/78454
>From 3a1ef6006764bd4d307ceec74199ed81a18aba2d Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-and...@users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd]
https://github.com/HighCommander4 approved this pull request.
Thanks, LGTM!
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/tom-anders updated
https://github.com/llvm/llvm-project/pull/78454
>From 3a1ef6006764bd4d307ceec74199ed81a18aba2d Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-and...@users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd] forward
tom-anders wrote:
> Something else I noticed while trying out the patch locally: before the
> patch, the description of the code action in the editor is "change 'foo' to
> 'Foo'", i.e. a description of what the code action will do.
>
> After the patch, the description of the code action is
@@ -648,6 +649,27 @@ tweakSelection(const Range , const InputsAndAST ,
return std::move(Result);
}
+// Some fixes may perform local renaming, we want to convert those to clangd
+// rename commands, such that we can leverage the index for more accurate
+// results.
HighCommander4 wrote:
(It's also no longer labelled as a "quick fix", likely due to the
`CodeActionKind` changing from `quickfix` to `refactor`, but that's probably
fine.)
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
HighCommander4 wrote:
Something else I noticed while trying out the patch locally: before the patch,
the description of the quick fix in the editor is "change 'foo' to 'Foo'", i.e.
a description of what the code action will do.
After the patch, the description of the quick fix is now "invalid
@@ -648,6 +649,27 @@ tweakSelection(const Range , const InputsAndAST ,
return std::move(Result);
}
+// Some fixes may perform local renaming, we want to convert those to clangd
+// rename commands, such that we can leverage the index for more accurate
+// results.
@@ -648,6 +649,27 @@ tweakSelection(const Range , const InputsAndAST ,
return std::move(Result);
}
+// Some fixes may perform local renaming, we want to convert those to clangd
+// rename commands, such that we can leverage the index for more accurate
+// results.
https://github.com/HighCommander4 edited
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -648,6 +649,27 @@ tweakSelection(const Range , const InputsAndAST ,
return std::move(Result);
}
+// Some fixes may perform local renaming, we want to convert those to clangd
+// rename commands, such that we can leverage the index for more accurate
+// results.
https://github.com/HighCommander4 requested changes to this pull request.
Ok, I had a more detailed look at the implementation. Still looks good overall,
just have some minor comments.
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits
https://github.com/tom-anders updated
https://github.com/llvm/llvm-project/pull/78454
>From 0f4be9fe945a23598c6eb49d733e0a709375669d Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-and...@users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd] forward
tom-anders wrote:
(fixed formatting issues)
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
tom-anders wrote:
> Looks pretty good!
>
> Are there further changes you're planning to make, or is this ready to
> graduate from "Draft" status?
Ah, thanks about the heads up, forgot about this
https://github.com/llvm/llvm-project/pull/78454
___
llvmbot wrote:
@llvm/pr-subscribers-clangd
Author: Tom Praschan (tom-anders)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/78454.diff
10 Files Affected:
- (modified) clang-tools-extra/clangd/ClangdLSPServer.cpp (+32)
- (modified)
https://github.com/tom-anders ready_for_review
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
HighCommander4 wrote:
Looks pretty good!
Are there further changes you're planning to make, or is this ready to graduate
from "Draft" status?
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/tom-anders updated
https://github.com/llvm/llvm-project/pull/78454
>From 579d681323db5f92d494f0cd0aaa9158dc8c4e3b Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-and...@users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd] forward
tom-anders wrote:
Thanks for the feedback!
> Thanks, the approach in this patch looks pretty good to me.
>
> My only feedback is to encapsulate the "hard coding" into a function like:
>
> ```
> Option TryConvertToRename(const Diag *D, const Fix
> *F)
> ```
>
> because I can imagine in the
HighCommander4 wrote:
> Duplicate issue:
> [clangd/clangd#741](https://github.com/clangd/clangd/issues/741)
Nice find, I closed it as a duplicate.
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
tom-anders wrote:
Duplicate issue: https://github.com/clangd/clangd/issues/741
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/HighCommander4 commented:
Thanks, the approach in this patch looks pretty good to me.
My only feedback is to encapsulate the "hard coding" into a function like:
```
Option TryConvertToRename(const Diag *D, const Fix *F)
```
because I can imagine in the future coming across
HighCommander4 wrote:
Linking to the issue this is seeking to address for reference:
https://github.com/clangd/clangd/issues/1589
https://github.com/llvm/llvm-project/pull/78454
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/tom-anders updated
https://github.com/llvm/llvm-project/pull/78454
>From 1fb6fb163bf2e63cb30571b179768bae2f43eb71 Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-and...@users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd] forward
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 837cde8aa15b0eb17aefc24530e82e25000d9fe0
71619f8dc07ca038b8e2e30d89f308c1e38a0726 --
https://github.com/tom-anders created
https://github.com/llvm/llvm-project/pull/78454
None
>From 71619f8dc07ca038b8e2e30d89f308c1e38a0726 Mon Sep 17 00:00:00 2001
From: tom-anders <13141438+tom-and...@users.noreply.github.com>
Date: Wed, 17 Jan 2024 15:36:02 +0100
Subject: [PATCH] [clangd]
28 matches
Mail list logo
