Skip to content

fix: include description in label details when detail field is marked for … #18245

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 9, 2024
Merged

fix: include description in label details when detail field is marked for … #18245

merged 1 commit into from
Oct 9, 2024

Conversation

boattime
Copy link
Contributor

@boattime boattime commented Oct 5, 2024

Fixes #18231.

When omitting the autocomplete detail field, the autocomplete label details can still be returned. Currently the label details are missing the description field if the detail field is included in resolveSupport since it is being overwritten as None and opted to be sent with completionItem/resolve.

Example completion capabilities.

completion = {          
    completionItem = {            
        commitCharactersSupport = true,            
        deprecatedSupport = true,            
        documentationFormat = { "markdown", "plaintext" },            
        insertReplaceSupport = true,            
        insertTextModeSupport = {              
            valueSet = { 1, 2 }            
        },            
        labelDetailsSupport = true,            
        preselectSupport = true,            
        resolveSupport = {              
            properties = { "documentation", "detail", "additionalTextEdits", "sortText", "filterText", "insertText", "textEdit", "insertTextFormat", "insertTextMode" }            
        },            
        snippetSupport = true,            
        tagSupport = {              
            valueSet = { 1 }            
        }          
}

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 5, 2024
@boattime boattime changed the title Include description in label details when detail field is marked for … fix: include description in label details when detail field is marked for … Oct 5, 2024
@jhgg jhgg mentioned this pull request Oct 5, 2024
Merged
bors added a commit that referenced this pull request Oct 9, 2024
@bors
Auto merge of #18247 - jhgg:lsp/fix-something-to-resolve, r=Veykril
7692c82 
lsp: fix completion_item something_to_resolve not being a latch to true

while looking at #18245 i noticed that `something_to_resolve` could technically flap between true -> false if some subsequent fields that were requested to be resolved were empty.

this fixes that by using `|=` instead of `=` when assigning to `something_to_resolve` which will prevent it from going back to false once set.

although some cases it's simply assigning to `true` i opted to continue to use `|=` there for uniformity sake. but happy to change those back to `=`'s.

cc `@SomeoneToIgnore`
@boattime
Copy link
Contributor Author

boattime commented Oct 9, 2024

@Veykril could you review?

davidbarsky
davidbarsky approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Would be good to have snapshot test in the future, since I don't think we have those right now.

(Lukas is at EuroRust this week and is preparing a talk.)

@davidbarsky
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 9, 2024

📌 Commit b857a0b has been approved by davidbarsky

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 9, 2024

⌛ Testing commit b857a0b with merge 0fb804a...

@bors
Copy link
Contributor

bors commented Oct 9, 2024

☀️ Test successful - checks-actions
Approved by: davidbarsky
Pushing 0fb804a to master...

@bors bors merged commit 0fb804a into rust-lang:master Oct 9, 2024
11 checks passed
@boattime
Copy link
Contributor Author

boattime commented Oct 9, 2024

@davidbarsky Thanks, I can submit tests shortly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbarsky davidbarsky davidbarsky approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RA nightly starting from v0.4.2130 doesn't show method signatures in completion popup window
4 participants
@boattime @davidbarsky @bors @rustbot