[PATCH] D151294: [clangd] Remove inline Specifier for DefineOutline Tweak

[Brian Gluzman via Phabricator via cfe-commits]

bgluzman added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:137
+llvm::Expected<tooling::Replacements>
+getDelKeywordReplacements(const FunctionDecl *FD,
+                          const syntax::TokenBuffer &TokBuf,
----------------
kadircet wrote:
> let's just drop the `FD` from signature, you can get `SourceManager` from `syntax::TokenBuffer`.
> 
> also can you update the documentation, we should rather say:
> ```
> Returns replacements to delete tokens with kind `Kind` in the range `FromRange`.
> ```
> 
> nit: s/getDelKeywordReplacements/deleteTokensWithKind
Getting it from `syntax::TokenBuffer` is a lot nicer, thanks!


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp:489
+    tooling::Replacements HeaderUpdates;
     const tooling::Replacement DeleteFuncBody(
         Sel.AST->getSourceManager(),
----------------
kadircet wrote:
> you can directly construct `Replacements` with this particular `Replacement` first, that way no need to `add` and check for `Err`
That's much cleaner than doing this with `add`, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151294