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
x/tools/internal/lsp: consider removing constant value from completion candidates #31923
Comments
It would be nice to keep the value in the |
Are you referring to the "detail" field of the completion candidate (the "float64", "int", "string" info in my screenshot), or do you mean the hover details, or something else? |
The detail field of the completion candidate. |
Do you have an example of when that would be useful? For me that would be almost equivalent to being in the label since the detail shows up right next to the label. |
In vim-go the detail field is used to populate the info property, which is then used to populate the preview window. |
Can you give a screenshot of the preview window? I'm not familiar enough with vim to know what that refers to. Do you have an example of a constant where it is useful to see its value in the preview window? |
Thanks, that is helpful. It doesn't look like the details are currently showing up at all for you, though. In my screenshot, the Go types that show up are the candidate "detail". The function/constant/method indicator is the candidate "kind". Anyway, I agree that the label should reflect what will get inserted if you select that completion. I'm still not sure there is a use case for showing constant values in the detail, but if that is something people want we can always add an "expanded detail" config param.
The detail for functions already contains the return values. See my below screenshot. Having the label followed by the details gives you the signature (minus the receiver). |
@stamblerre what do you think about constant values in completion candidates? |
The |
gopls can simply this a lot by making the details be more complete description instead of just what the item evaluates to. But I feel like this is perhaps digressing into an issue that I've been intending to create to rethink the details field generally. Sorry for that; I can create a separate issue if you'd like to talk about the wider issues so we can keep this focused on how to manage the details field for const types. |
Haha, sorry. Didn't notice. What you are saying makes a lot more sense having the details and labels separated like that.
Yeah I agree there are lots of cool/useful things we could do. We could also consider adding any extra detailed info into the "Documentation" field in addition to any actual docs, but that might not make sense.
Yes, please. I'd like to start with removing constant values from the label if no one objects to that in particular. |
I had the constant values in the label because it's something we have internally in Google, and I hadn't heard any complaints. Do you think it would be worth keeping it in the detail? I am not sure how it would look there, since in VSCode, detail is on the right of the label. I would support removing it if we can't imagine a single case where it might be useful. I can only think that maybe with string constants it could be helpful? |
I think would be pretty equivalent to being in the label since (in most editors) the detail shows up right next to the label, albeit somewhat deemphasized.
Yeah, I could imagine a case where you are composing a bigger string using string constants and it might be useful, but I can't think of any real examples. Regardless, if 99/100 times the constant value is extra noise, I'd still be in favor of getting rid of it. |
Absolutely agreed. Let's remove it, and if anyone complains and offers a valid counter example, perhaps we can consider adding a special case for it. |
Change https://golang.org/cl/177357 mentions this issue: |
Currently completion candidate labels for constants include the constant value. I personally find this distracting since the value of a constant is almost never material when choosing completions (i.e. you choose based on the constant name, not the value).
I polled the Slack #tools channel and everyone who responded agreed with this sentiment. Therefore, I propose we remove the constant's value from the label.
Also, the label is especially unhelpful for floating point constants:
The text was updated successfully, but these errors were encountered: