https://github.com/HighCommander4 requested changes to this pull request.
Thanks for putting together this patch! Two high-level pieces of feedback: 1. The current patch implements the options as additional values of the `--function-arg-placeholders` command-line flag. However, the proposal in [this comment](https://github.com/llvm/llvm-project/issues/63565#issuecomment-1975065771) was to make these values of a new config file key called `ArgumentLists`, under [`Completion`](https://clangd.llvm.org/config.html#completion). (This involves a bit of boilerplate; it helps to look at a previous patch which adds a new config key, such as [this one](https://github.com/llvm/llvm-project/commit/16fd5c278488b3d3275afc381a00ba0b51b070ee).) 2. It would be great to have some unit tests exercising the options. We have an existing unit test exercising the current boolean flag [here](https://github.com/llvm/llvm-project/blob/587d4cc797e09eeb2ec93d8b4ee9cd484626bf8f/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#L2598) which can be extended as appropriate. https://github.com/llvm/llvm-project/pull/108005 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
