zed-industries
lsp: Fix signature help parameter highlighting
This update enhances the parameter highlighting logic in the SignatureHelp. Previously, SignatureHelp displayed parameters separated by commas, which was incorrect for languages like Scheme. This commit resolves the issue by displaying the full signature, aligning the behavior with editors such as VS Code.
Closes: #22128
Release Notes:
- Improved parameter highlighting in signature help.
We require contributors to sign our Contributor License Agreement, and we don't have @roife on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.
![cla-bot[bot] avatar](https://avatars.githubusercontent.com/in/2404?v=4 "Published by a pub.dev verified publisher"){kind=small}
@roife In the screenshot you posted, I think the previous behavior is better for rust. It shows you the parameter you need to type.
Is there a way we can fix the S-expressions case without making the UI noisier for languages that do use comma separators?
@roife In the screenshot you posted, I think the previous behavior is better for rust. It shows you the parameter you need to type.
I agree that the original approach here looks more concise. However, I think this is a tricky behavior that may only be suitable for languages like Rust/C++/etc. Other languages might include additional useful information in the function signature, which would all be stripped away here.
The reason I changed it to the new approach is that it aligns with other common editors like VS Code, NeoVim, and Emacs. I believe we need to provide users with a standard behavior that is the same as other editors.
I'm not sure if providing an additional option for users to choose between the full signature and the simplified signature would be appropriate, as it might make the configuration look too complex?
Is there a way we can fix the S-expressions case without making the UI noisier for languages that do use comma separators?
Perhaps we can handle those languages specially.
I definitely agree that this doesn't need configuration. I also think aesthetically we don't need to repeat the function name because you just typed it, and it's visible to the left.
It seems like we could improve on the naive "join with a comma" by instead extracting the substring from the overall label that represents all the parameters (or by finding the piece "between" the first two parameters and using that everywhere).
It's not super clear what editors that show the full label are supposed to do when the "string"-based parameter format is used. I think "find the matching index" is probably about as good as we can do, though it's annoyingly fiddly.
To note, https://github.com/zed-industries/zed/pull/33199 brings the same notion and I tend to agree that it's lengthier but somewhat better because provides a return type: inventing another pseudo syntax for showing that in our form seems odd.
So, for a bit simpler reasoning, I'd use what most of the other editors show in the signature help popovers (the full form), and iterate on that for cases like Rust: we might use something better than a popover (e.g. inlays) to show things like return type and parameters.
