[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

kadircet added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > sammccall wrote: > > > > kadircet wrote: > > > > >

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

kadircet updated this revision to Diff 166076. kadircet marked 14 inline comments as done. kadircet added a comment. - Delete include fixit option. - Rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51214 Files: clangd/tool/ClangdMain.cpp Index:

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added a comment. Maybe commit only an option to enable function arg snippets for now (found myself wanting this option today :-))? The fixes would also be nice, but since they never work... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51214

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:205 +static llvm::cl::opt EnableFunctionArgSnippets( +"enable-function-arg-snippets", +llvm::cl::desc("Gives snippets for function arguments, when disabled only " sammccall

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

kadircet added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > I wonder if we should make the `IncludeFixIts` option

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added a comment. +1 to adding an option to drop arguments from snippets and removing the option for the fixes. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", sammccall wrote: >

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

sammccall added a comment. I think I'm still where I was a few weeks ago - option to drop args makes sense, completions with fixes isn't something users should care about. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits",

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

kadircet updated this revision to Diff 164847. kadircet marked 4 inline comments as done. kadircet added a comment. - Update descriptions and change parameter name. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51214 Files: clangd/tool/ClangdMain.cpp Index:

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > ilya-biryukov wrote: > > > > I wonder if we should

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

sammccall added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > I wonder if we should make the `IncludeFixIts` option

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added a comment. LG from my side, @sammccall is not a big fan of options so please wait for his approval too Comment at: clangd/tool/ClangdMain.cpp:174 +llvm::cl::desc( +"Enables suggestion of completion items that needs additional changes. " +

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

kadircet updated this revision to Diff 164830. kadircet marked 5 inline comments as done. kadircet added a comment. - Change flag's name and rebase. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51214 Files: clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:202 +"Like changing an arrow(->) member access to dot(.) member access."), +llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts)); + sammccall wrote: > ilya-biryukov

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", sammccall wrote: > ilya-biryukov wrote: > > I wonder if we should make the `IncludeFixIts` option hidden? > > It

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

sammccall added a comment. The description says "LSP clients", i.e. editors. For editors (e.g. whether editor can handle snippets), LSP capability negotiation rather than flags is the right thing. I suspect this is true for including completion fixes. For users (e.g. I like behavior X), flags

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

ilya-biryukov added a subscriber: sammccall. ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:197 +static llvm::cl::opt IncludeFixIts( +"include-fixits", I wonder if we should make the `IncludeFixIts` option hidden? It currently

[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

kadircet created this revision. kadircet added reviewers: hokein, ioeric, ilya-biryukov. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Currently LSP clients cannot directly change IncludeFixIts or EnableFunctionArgSnippets parameters. This patch is to provide them with a way