hokein added inline comments.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1035
}
+ if (KindAllowed(CodeAction::SOURCE_ORGANIZE_IMPORT)) {
+ std::lock_guard<std::mutex> Lock(FixItsMutex);
----------------
kadircet wrote:
> instead of doing this in here, what about introducing a new "tweak" that'll
> perform include-cleaner fixes?
> Pros are:
> - We can use it in places that don't use LSPServer.
> - We can later on extend "organize imports" behaviour to do other things if
> needed.
> - Results will be always up-to-date, as we can use the AST and compute the
> fixes. rather than making use of the "latest" cached version.
> - Implementation would be a little bit neater (and contained), as we can just
> re-use the existing components in IncludeCleaner, rather than doing some
> ad-hoc matching & retrieval.
>
> WDYT?
I think it is a matter of how we view it (whether it is a fix for all
diagnostics or a tweak).
but +1, yeah, this looks like a good idea to me.
================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1072-1074
+ AddFix(RemoveAllUnused);
+ AddFix(AddAllMissing);
+ AddFix(FixAll);
----------------
kadircet wrote:
> what does the UI look like when there are multiple code actions with
> "organize imports" kind?
> e.g. 1&2 can actually be applied together, but 3rd one can't be combined with
> others, and in theory these can also be triggered on save (if the user opts
> in). so I am not sure if having multiple code actions, especially when they
> can't be merged together, really makes much sense on the editor side.
> what does the UI look like when there are multiple code actions with
> "organize imports" kind?
Similar to the normal code action, if you trigger it via the right context menu
(`SourceAction`), a window with 3 alternatives will pop up, so that you can
select one of them (verified on VSCode).
> e.g. 1&2 can actually be applied together, but 3rd one can't be combined with
> others, and in theory these can also be triggered on save (if the user opts
> in). so I am not sure if having multiple code actions, especially when they
> can't be merged together, really makes much sense on the editor side.
Good point. The VSocde `codeActionsOnSave` behavior is like what you said --
it applies each fix one by one (because we trigger preview window per fix, so 3
preview windows will be triggered in sequence).
I don't see the best way to fix it (from the `codeAction` requests are
identical from the one sent via the context menu), one option is to send only a
`fixAll` codeAction rather than 3.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142967/new/
https://reviews.llvm.org/D142967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
