Skip to content

Conversation

@lnicola
Copy link
Member

@lnicola lnicola commented Dec 21, 2020

Closes #2042

We're now using the CompletionItem::tags field to mark CompletionItems as deprecated, and CompletionItem::deprecated is gone from LSP.

@matklad
Copy link
Contributor

matklad commented Dec 21, 2020

Hm, I don't quite understand why :) Reading the linked issue doesn't help. Could you ammend the commit message perhpars to say why, not what? ;)

@lnicola
Copy link
Member Author

lnicola commented Dec 21, 2020

Updated (only the description, since bors will copy it into the merge commit anyway).

@matklad
Copy link
Contributor

matklad commented Dec 21, 2020

bors r+

though, I personally still prefer to put info in first-order commit messages -- would work better with blame that way I think

@bors
Copy link
Contributor

bors bot commented Dec 21, 2020

@bors bors bot merged commit 7843ae6 into rust-lang:master Dec 21, 2020
@lnicola lnicola deleted the deprecated-deprecated branch December 21, 2020 12:18
@kjeremy
Copy link
Contributor

kjeremy commented Dec 21, 2020

This is incorrect. The field isn't gone but deprecated in the protocol. It still needs to be set for older clients.

@lnicola
Copy link
Member Author

lnicola commented Dec 21, 2020

But the field still shows up in the response, I assume lsp_types does that?

And we might have other compatibility issues anyway (like in #6779).

@lnicola
Copy link
Member Author

lnicola commented Dec 21, 2020

[Trace - 15:47:57] Received response 'completionItem/resolve - (48)' in 1ms.
Result: {
    "label": "foo()",
    "kind": 3,
    "detail": "fn foo()",
    "deprecated": true,
    "filterText": "foo",
    "insertTextFormat": 2,
    "textEdit": {
        "range": {
            "start": {
                "line": 4,
                "character": 4
            },
            "end": {
                "line": 4,
                "character": 6
            }
        },
        "newText": "foo()$0"
    },
    "additionalTextEdits": [],
    "tags": [
        1
    ]
}

@kjeremy
Copy link
Contributor

kjeremy commented Dec 21, 2020

Both the field (for older) and the tag (newer) should be set. The tag was introduced to the protocol to stop stuffing new info into fields. There are a number of LSP requests that have started to follow this model.

It's deprecated in the sense that newer clients should be looking at the tag set and not the field to determine deprecation status but they should both be filled out. When a new thing is added in the future it will be added to the tag list and not made a field. There shouldn't be any compatibility problems with this approach.

bors bot added a commit that referenced this pull request Dec 21, 2020
6977: Revert "Stop setting CompletionItem::deprecated" r=lnicola a=lnicola

We should keep setting it, according to #6974 (comment).

Co-authored-by: Laurențiu Nicola <lnicola@dend.ro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecation for completion items

3 participants