Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trigger signature help after accepting a function completion. #390

Closed
MK-Alias opened this issue Oct 12, 2022 · 32 comments · Fixed by #398
Closed

Trigger signature help after accepting a function completion. #390

MK-Alias opened this issue Oct 12, 2022 · 32 comments · Fixed by #398
Labels
enhancement New feature or request

Comments

@MK-Alias
Copy link

Goodday,

I really enjoy using clangd. Its a must-have tool for every C++ programmer. However, there is one things that really irritates me. Because just as the title implies, it does not give a hover on auto-completing functions/metods. The default is to insert placeholders. Which honestly is weird for an extreme pedant language like C++ to insert illegal syntax (and also spamming the output on functions with a long agrument list). Fortunally it can be turned of with adding this argument.

--function-arg-placeholders=0

This will however break VScode's hover -- which is a way better way to use completing arguments all to getter. So now when auto-completing a function/method it will just type out the function with paranthesis. Regardless of howmany arguments it has. If you want to use the hover you then must press backspace to remove the parenthesis and then retype to parenthesis. Which is irritating me on a daily basis.

Also clangd is not honoring my parenthesis preference with this!

A nice solution whoud be to just not insert the parenthesis. This will force me (which is also conform my preference) to type the parenthesis myself and with that triggering the hover. But there are probably other things you can do inside the VScode plugin API to make the hover show if not using function-arg-placeholders

Thanks for your time for reading this,
and please make people like me a bit happier in this world by given us the hover!

Thank You!

--
using: clangd version 14.0.6
editor: VScode
arguments:

"clangd.arguments": [
    "--function-arg-placeholders=0",
    "--header-insertion=never",
]
@MK-Alias MK-Alias added the enhancement New feature or request label Oct 12, 2022
@sam-mccall
Copy link
Member

This is a bit hard to follow, but IIUC there are two complaints:

  1. you want a mode where function completion inserts foo rather than foo() or foo(int x).
  2. In VSCode, after accepting a function code completion, signature help doesn't immediately trigger.

The first is actually something clangd does, but we only enable it if the editor doesn't support rich "snippet completions" (e.g. "insert foo() but place the cursor between the parens). I think this isn't exposed as an option just because of our perception people don't want it. Even here it's not clear to me if it's important per se or just a workaround for the second problem.

The secord problem is a VSCode-specific limitation/bug, this works OK in other editors. VSCode says the extension should provide an explicit hint to do this (microsoft/vscode#31290) and the owners of the VSCode LSP library + LSP itself seem to think this hint should come via LSP, but haven't added it to LSP yet. (microsoft/language-server-protocol#274). I think we can work around this bug on the client-side by injecting the hint ourselves in the middleware.

I'm going to transfer this to the vscode-clangd repo since the second issue seems more important and is vscode-specific.

@sam-mccall sam-mccall transferred this issue from clangd/clangd Oct 12, 2022
@sam-mccall sam-mccall changed the title Please show hover on a function/method auto-complete! Trigger signature help after accepting a function completion. Oct 12, 2022
@sam-mccall
Copy link
Member

Plan for this would be:

  • intercept completion items in the middleware (we already do this)
  • find items where the cursor is placed after a signature help trigger character (or maybe do this unconditionally?)
  • set completion item command to editor.action.triggerParameterHints

@wtdcode
Copy link

wtdcode commented Oct 20, 2022

I have a similar problem like this: https://discourse.llvm.org/t/automatic-parameter-suggestions/2419

Basically, given a function:

int var;

void some_function(int a, int b)

type some<tab> will soon give:

some_function(<pointer here>int a, int b)

typing v will become:

some_function(v, int b)

However, no autocompletion for var pops, which is quite annoying...

sam-mccall added a commit to sam-mccall/vscode-clangd that referenced this issue Oct 22, 2022
…iate.

As recommended in microsoft/vscode#78806
This is an incomplete fix but better than nothing.
Also files microsoft/vscode#164310 in case
VSCode can improve the behavior here.

fixes clangd#390
@sam-mccall
Copy link
Member

@wtdcode that is a different issue and I don't think one we can do anything about it, please file a bug against vscode.

I have https://github.com/clangd/vscode-clangd/pull/398/files for the original issue.

@wtdcode
Copy link

wtdcode commented Oct 22, 2022

@wtdcode that is a different issue and I don't think one we can do anything about it, please file a bug against vscode.

I have https://github.com/clangd/vscode-clangd/pull/398/files for the original issue.

I don't think it's a vscode bug as other plugins work well in similar cases. Maybe I can open a separate issue here?

@MK-Alias
Copy link
Author

@wtdcode that is a different issue and I don't think one we can do anything about it, please file a bug against vscode.

Yes, but the reason for that is that you're not using the reguar signature-help. Instead - for what ever reason - you desided to go with the snippet engine for functions/methods which is not the way the editor is intended to work. No other language does this. This is your design choice, not that of VScode.

Maybe you can make an option to either use the [currently used] snippet-engine or the [requested] signature-help. But honestly I think the snippet-engine is inferieur in every way. Just look at @wtdcode issue and the fact it is generating illegal syntax and just a lot of spam on long argument lists, and lastly also not showing the function/method documentation (comment above the function/method).

@MK-Alias
Copy link
Author

It has been nine months, is there any hope that this will be resolved? Any change that the snippet engine will be dropped in favor of the regular signature help? -- which is the intended VScode way and used by all other languages! Or are we going to live with the fact that this novel way of using the snippet engine for signature help will never change!?

clangd is a really nice VScode extension and really helps me out a lot, but it would be a lot more usable of it would just confirm to the standard way of doing this.

Thank you!

@HighCommander4
Copy link
Contributor

Having read through the discussion here, I have a few questions:

@sam-mccall: The PR you wrote and linked to above is approved. Is it ready to be merged, or is it waiting on something else?

@MK-Alias: Would Sam's PR resolve the issue to your satisfaction? Based on your last two comments I get the impression you're looking for a different solution, but it's not yet clear to me what that would look like, so any additional details would be appreciated.

For what it's worth, I did try a different language server, rust-analyzer, and the behaviour I see there is:

  • Code completion of a function call does insert placeholders (i.e., as far as I can tell, at the LSP level it does use the "snippet" capability), though the placeholder text consists only of the parameter name, not "parameter type and parameter name" the way clangd does. (This change has been suggested for clangd as well in Add option to disable parameter types from call completion snippet clangd#1252).)
  • In addition to the placeholders, a popup appears showing the function signature, with the current parameter highlighted, and pressing Tab simultaneously advances the cursor position in the editor to the next placeholder, and highlights the next parameter in the popup.
    • I think this is what Sam's patch would accomplish as well, but maybe I'm missing something.

@HighCommander4
Copy link
Contributor

@wtdcode that is a different issue and I don't think one we can do anything about it, please file a bug against vscode.
I have https://github.com/clangd/vscode-clangd/pull/398/files for the original issue.

I don't think it's a vscode bug as other plugins work well in similar cases. Maybe I can open a separate issue here?

Assuming by "other plugins" you mean "other vscode language server plugins for different languages" -- yes, please file a separate issue in this repo, and please mention a specific other-language plugin so we can play around with it and see how it operates at the LSP level.

@MK-Alias
Copy link
Author

@HighCommander4 Thanks for time of reading into this and for your responses. On your question if Sam's PR request will fix it, i can only say that i don't know because I don't know how to apply this patch to my local version. I'm willing to try if you can provide some documentation on this.

Here is a detailed description of whats going on and what my frustrations with this things are. Lets use this example code:

#include <iostream>
#include <string>

// simple example, compile with: clang++ speak.cpp

class speak {
public:
    /// say something, use times to set howmay times it needs to be say'd,
    /// set show_iterations to true to show number, and string &s for the message to say
    static void say(int times, bool show_iteration, const std::string &s) {
        for (int i = 0; i < times; i++) {
            std::cout << (show_iteration ? std::to_string(i) + ": " : "") << s << std::endl;
        }
    }
};

int main(int argc, char *argv[]) {
    speak::say(1,true,"hello");
    return 0;
}

When using only: --header-insertion=never in my clangd.arguments. (which should not interfere with these examples). I get the following results if i'm not using autocomplete, Lets call this method A:

shots_vanila_no-complete

This will result in VScode "signature help" solution. No placeholders or "snippet like" shenanigans.

Now, if you would use autocomplete (using the tab key) I get the next result, lets call this method B:

shots_vanila_autocomplete

This will result in a "snippet like" solution where everything is different than the "signature help" solution. I really don't like this method. Because it does not show any documentation (comments above functions), the tab cycling irritates me and lastly, if you have a function with lots of arguments that consists out of classes being referred in namespaces/nesting-classes this really becomes unreadable/spam and in my opinion unusable.

Now, if you would add the argument --function-arg-placeholders=0 to the clangd.arguments and would use autocomplete we get this, lets call this method C

shots_arg0_autocomplete

Now, the placeholders "snippet like" shenanigans is gone! (yay!). But the clangd executable now adds () on the end of the function, which is unwanted! After removing the ( and adding a new ( OR entering the first argument, the "signature help" spawns and im happy. But to use this, I must always press backspace and ( after doing auto-completion, which is annoying.

The desired way of doing this, would be method C but then without inserting the '()`, which will look like this: method D. (this is fabricated by me, not a real recording).

shots_desired

in order to to this you simply need to remove the () when auto completion is done. In pseudo-code it is something like this.

void onAutocompleteFinished() {
    if (function-arg-placeholders == 0) {
        removeLeftChar();
        removeLeftChar();
    }
}

Some general frustration with all of this is that you are not committing to one solution. It all depends on using auto-completion or not, and if --function-arg-placeholders=0 is set. Which all will lead to different solutions. Which indicates that this is not thoroughly thought thru.

A quickfix would be something like my psuedo-code solution and perhaps Sams patch already does this (as stated i haven't check that out). A better solution would be to make this as an option in the clangd-vscode extension so that users can choose their own preferences on this. We are not all the same, i really love the "signature help" solution and absolutely hate the "snippet like" solution, others might disagree and love the "snippet like" solution. For that reason it seems appropriate to let it be a preference.

I hope with this everything has become a lot more clear. I really love clangd and it improves my productivity a lot, but this needs to be addressed!

@HighCommander4
Copy link
Contributor

HighCommander4 commented Jul 24, 2023

Thanks for the details.

With Sam's patch and --function-arg-placeholders=0, the behaviour when selecting a completion proposal for a function call will be:

  • Parentheseses are inserted, with nothing between them (no placeholders)
  • Signature help is activated automatically

Does that work for you?

@HighCommander4
Copy link
Contributor

Or to illustrate with a screen recording:

vscode-test-1690184469327.mp4

@MK-Alias
Copy link
Author

@HighCommander4 that would be a big step in the right direction for sure! However, I rather have it that the parenthesis are not inserted. For reasons:

  • Sometimes we just need a function pointer (or std::function value) in which case you still have to remove the parenthesis, and don't need the signature help, although the documentation can be helpful here.
  • By having the closing parenthesis on the end, you always needs to skip them, which means that i have to move my hand to the arrow-section of my keyboard, while typing ) is a lot faster! (I'm not using vim-mode :)
  • I simply just don't like editors inserting anything else then auto-completing identifiers/types on demand.

@MK-Alias
Copy link
Author

Or to illustrate with a screen recording:
vscode-test-1690184469327.mp4

I cant see this video. Firefox says it's corrupt!

@HighCommander4
Copy link
Contributor

@HighCommander4 that would be a big step in the right direction for sure! However, I rather have it that the parenthesis are not inserted. For reasons:

  • Sometimes we just need a function pointer (or std::function value) in which case you still have to remove the parenthesis, and don't need the signature help, although the documentation can be helpful here.

  • By having the closing parenthesis on the end, you always needs to skip them, which means that i have to move my hand to the arrow-section of my keyboard, while typing ) is a lot faster! (I'm not using vim-mode :)

  • I simply just don't like editors inserting anything else then auto-completing identifiers/types on demand.

Fair enough.

I suggest we use this issue to track Sam's patch, which will address the issue as titled: "Trigger signature help after accepting a function completion".

For the remaining issue of adding an option to disable insertion of the parentheses, we have llvm/llvm-project#63565 on file, we can track it there.

@HighCommander4
Copy link
Contributor

Or to illustrate with a screen recording:
vscode-test-1690184469327.mp4

I cant see this video. Firefox says it's corrupt!

Yeah, I see the same thing. Right click --> Copy Video Link works as a workaround.

@wtdcode
Copy link

wtdcode commented Jul 24, 2023

Happy to know that there is some progress! I would like to add a point here: I have started using rust-analyzer plugin for a few months and I notice that when it inserts a function call and types something, auto-completion will be triggered as expected. I'm not VSCode expert but I think the ideal way is what rust-analyzer does.

@MK-Alias
Copy link
Author

@HighCommander4

But what about the fact that if you don't set: --function-arg-placeholders=0 which is the vanilla way of using clangd-vscode. Then users will get 'Signature Help' when not auto-completing and 'Snippets placeholders' when auto-completing. Surly you must agree that this is can/might be frustrating/annoying to users.!? It really seems that a reevaluation is whats needed, not just a quickfix. (although the quickfix would still make me happy!)

Also there is an editor option: editor.suggest.showSnippets which is not honored by clangd-vscode. Because mine is set to false, and i still get snippets (placeholders) by clangd-vscode.

The main problem lays in the clangd executable, that's why i asked this question there first before @sam-mccall transferred it here . --function-arg-placeholders=0 should not insert the () to begin with. However there might be dependency on this now (by other editors/clients). This can still be fixed in clangd executable by putting a new option, something like: --function-arg-placeholders=empty or --function-arg-placeholders=-1 in which case clangd will not append the (). Personally I think that would be a nicer option, which other editors/clients might also enjoy!

A bit off-topic, but having some nice checkboxes for these options instead of putting them in a clangd.arguments array would also be a lot more user friendly.

  • --function-arg-placeholders=0
  • --header-insertion=never

User friendliness would improve a lot with that. I can remember heaving to figure out that you can set these options in clangd.arguments without any documentation. I was a bit of a pain!

@HighCommander4
Copy link
Contributor

But what about the fact that if you don't set: --function-arg-placeholders=0 which is the vanilla way of using clangd-vscode. Then users will get 'Signature Help' when not auto-completing and 'Snippets placeholders' when auto-completing. Surly you must agree that this is can/might be frustrating/annoying to users.!? It really seems that a reevaluation is whats needed, not just a quickfix. (although the quickfix would still make me happy!)

Sam's patch makes it so that signature help is also shown (along with placeholders) when --function-arg-placeholders=0 is not present.

So, the behaviour after Sam's patch seems pretty orthogonal to me:

  • Signature help is always shown when accepting a completion proposal for a function call
  • Placeholders are shown or not depending on the --function-arg-placeholders option

Also there is an editor option: editor.suggest.showSnippets which is not honored by clangd-vscode. Because mine is set to false, and i still get snippets (placeholders) by clangd-vscode.

In that option, "snippets" refers to completion proposals whose LSP CompletionItemKind is Snippet. This is used for "code snippets" like for loops, while loops, try/catch blocks, etc. The option governs whether completion proposals for such code snippets are offered at all.

--function-arg-placeholders=0 should not insert the () to begin with.

"Placeholders" refers specifically to the parts of the completion proposal for which some placeholder text is inserted, which is expected to be replaced by the user (and the editor provides features such as e.g. Tab-bing between the placeholders). For a function call, the placeholders are the function arguments, and do not include the parentheses.

So, I believe the current behaviour is consistent with the name of this option.

Of course, that doesn't preclude adding a different option to govern the insertion of parentheses, which is what llvm/llvm-project#63565 tracks.

However there might be dependency on this now (by other editors/clients). This can still be fixed in clangd executable by putting a new option, something like: --function-arg-placeholders=empty or --function-arg-placeholders=-1 in which case clangd will not append the (). Personally I think that would be a nicer option, which other editors/clients might also enjoy!

One of the things we'll want to discuss in llvm/llvm-project#63565 is the exact form of the option to govern the insertion of parentheses.

We've been moving away from options specified as command-line arguments, and towards options specified in the clangd config file, so the new option may take that form.

A bit off-topic, but having some nice checkboxes for these options instead of putting them in a clangd.arguments array would also be a lot more user friendly.

  • --function-arg-placeholders=0

  • --header-insertion=never

User friendliness would improve a lot with that. I can remember heaving to figure out that you can set these options in clangd.arguments without any documentation. I was a bit of a pain!

This is a more general discussion, but as mentioned, we've been moving in the direction of consolidating our options in the clangd config file, which is an editor-independent place.

I don't think it scales to try and maintain editor-specific settings for our options, as that would involve a fair amount of repeated work between different editors.

Instead, I think we can focus on making the clangd config file more discoverable, and more friendly to edit (e.g. recently we've added a schema for it such that vscode offers auto-completion for the option names in the clangd config file).

@HighCommander4
Copy link
Contributor

I would like to add a point here: I have started using rust-analyzer plugin for a few months and I notice that when it inserts a function call and types something, auto-completion will be triggered as expected. I'm not VSCode expert but I think the ideal way is what rust-analyzer does.

Could I ask you to file a new ticket for this please, and include additional information (e.g. a code example, clangd logs, or a screencast as appropriate) so we can better understand the scenario?

It helps keep things organized to limit the scope of each ticket to one issue.

@MK-Alias
Copy link
Author

@HighCommander4

Thanks for all you feedback en vision on everything! I hope to see an update with Sam's patch in the not so far future.

@lhmouse
Copy link

lhmouse commented Mar 1, 2024

Have there been any updates about this issue? I think the suggestion is very reasonable.

@MK-Alias
Copy link
Author

MK-Alias commented Mar 1, 2024

Unfortunately removing the '()' insertion seems like a small request but 1,5 years later, this still is an issue!

@lhmouse
Copy link

lhmouse commented Mar 2, 2024

Unfortunately removing the '()' insertion seems like a small request but 1,5 years later, this still is an issue!

A solution might be https://marketplace.visualstudio.com/items?itemName=mitaki28.vscode-clang

But it's not a replacement for vscode-clangd; 'Go to definition' etc. from clangd are still essential, so it's better to install both. For auto-completion, results from both extensions will be seen, then you may select the one without (. The annoyance should go away.

@lhmouse
Copy link

lhmouse commented Mar 2, 2024

Okay to put it clear: Please add an option to disable auto-completion like any other plugins. vscode-clangd is good, and thank you, you will be appreciated, but the auto-completion is causing a lot of confusion to me.

@MK-Alias
Copy link
Author

MK-Alias commented Mar 2, 2024

@lhmouse tried using the other extension in combination with clangd. Unfortunately it goes way beyond my frustration levels. I've tried @sam-mccall patch, which is a small improvement, but still doesn't work as desired. The official MS c++ extension works pretty nice, i've just installed it and works exactly as I would like a language support to work, but i always discontinue its use after a while because of pour performance.., kind of typical, one extension has amazing performance (clangd) and the other one has the amazing UX (MS). So, choose your poison! I'm still hopeful with crossed fingers that some great day in the not to distance future, clangd will work just as nice as the MS extension! A guy can dream!

HighCommander4 pushed a commit that referenced this issue Mar 3, 2024
…iate. (#398)

As recommended in microsoft/vscode#78806
This is an incomplete fix but better than nothing.
Also files microsoft/vscode#164310 in case
VSCode can improve the behavior here.

fixes #390
@HighCommander4
Copy link
Contributor

@sam-mccall: The PR you wrote and linked to above is approved. Is it ready to be merged, or is it waiting on something else?

Since I haven't heard a response from Sam since asking this question last July, and since the PR in question was approved, I went ahead and merged it.

@HighCommander4
Copy link
Contributor

I've tried @sam-mccall patch, which is a small improvement, but still doesn't work as desired.

Could you elaborate on what doesn't work as desired with Sam's patch? Is the remaining issue just llvm/llvm-project#63565 (insertion of the parentheses), or is there something else?

@HighCommander4
Copy link
Contributor

Please add an option to disable auto-completion like any other plugins. vscode-clangd is good, and thank you, you will be appreciated, but the auto-completion is causing a lot of confusion to me.

Please file a new issue about that.

@MK-Alias
Copy link
Author

MK-Alias commented Mar 3, 2024

Could you elaborate on what doesn't work as desired with Sam's patch? Is the remaining issue just llvm/llvm-project#63565 (insertion of the parentheses),

Yes, it is the parentheses which are not desirable. Your proposal: llvm/llvm-project#63565 (comment) seems very desirable! I hope it gets approved!

@lhmouse
Copy link

lhmouse commented Mar 11, 2024

@MK-Alias

@lhmouse tried using the other extension in combination with clangd. Unfortunately it goes way beyond my frustration levels.

May I suggest another option, ccls? The behavior of auto-completion is controlled by ccls.completion.enableSnippetInsertion, which is off by default, so no special configuration is required! Have a try.

@MK-Alias
Copy link
Author

Thanks for the suggestion. However CCLS does not distribute binaries. Source is not easily (at least not by me) compiled on windows (my current desktop). I tried the chocolatey version, but it seems to be braindead. (only using 1.1 mb of ram with no cpu usage). The VScode extension does not seem to work, it doesn't do anything, not even an error that it does not work.

CCLS might be a nice alternative for unix desktops, but currently i'm on windows. I do change desktops sometimes, and will try it on linux when the time comes, but for now it doesn't work for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants