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

Literal autocompletion inserts an extra quote #39

Closed
johannesschaffer opened this issue May 9, 2024 · 7 comments · Fixed by #52
Closed

Literal autocompletion inserts an extra quote #39

johannesschaffer opened this issue May 9, 2024 · 7 comments · Fixed by #52
Assignees
Labels
enhancement New feature or request

Comments

@johannesschaffer
Copy link

johannesschaffer commented May 9, 2024

What happened?

When I accept an autocompleted Literal value it inserts an ' or " at the end

Steps to reproduce

Command = Literal['first', 'second']
s: Command = '' # accept the autocomplete with the caret inside the quotes - Result: 'first''

Relevant log output or stack trace

No response

Operating system

None

@johannesschaffer johannesschaffer added the bug Something isn't working label May 9, 2024
@InSyncWithFoo
Copy link
Owner

Can reproduce:

from typing import Literal


T = Literal['first', 'second']
v: T

v = ''
2024-05-09 18:34:12,233 [ 820111]   FINE - #c.i.p.l.i.c.Lsp4jServerConnector - --> PyrightLSDescriptor@project:
{
    "jsonrpc": "2.0",
    "id": "61",
    "method": "textDocument/completion",
    "params": {
        "context": { "triggerKind": 1 },
        "textDocument": { "uri": "file:///<project>/.py" },
        "position": { "line": 6, "character": 5 }
    }
}
2024-05-09 18:34:12,235 [ 820113]   FINE - #c.i.p.l.i.c.Lsp4jServerConnector - <-- PyrightLSDescriptor@project:
{
    "jsonrpc": "2.0",
    "id": "61",
    "result": {
        "items": [
            ...,
            {
                "label": "'second'", "kind": 21, "sortText": "03.9999.'second'",
                "textEdit": {
                    "range": {
                        "start": { "line": 6, "character": 4 },
                        "end": { "line": 6, "character": 6 }
                    },
                    "newText": "'second'"
                }
            }
        ],
        "isIncomplete": true
    }
}
2024-05-09 18:34:12,724 [ 820602]   FINE - #c.i.p.l.i.c.Lsp4jServerConnector - --> PyrightLSDescriptor@project:
{
    "jsonrpc": "2.0",
    "method": "textDocument/didChange",
    "params": {
        "textDocument": { "version": 48, "uri": "file:///<project>/.py" },
        "contentChanges": [
            {
                "range": {
                    "start": { "line": 6, "character": 5 },
                    "end": { "line": 6, "character": 5 }
                },
                "text": "second'"
            }
        ]
    }
}

I believe this is not a problem with the plugin (I overrode none of the default behaviours, as you can see here), but a problem with how PyCharm interprets the suggestions.

The first request was triggered by pressing Ctrl Space in the middle of the two quotes:

{
    "method": "textDocument/completion",
    "params": {
        // | 0 | 1 | 2 | 3 | 4 | 5 | 6 |
        // | v |   | = |   | ' | ' |
        "position": { "line": 6, "character": 5 }
    }
}

The corresponding response then gave back two items, both of which have the same textEdit range spanning from character 4 up to 6 (which presumably means both quotes are replaced) and a newText value having both leading and trailing quotes:

{
    "result": {
        "items": [
            ...,
            {
                "label": "'second'",
                "textEdit": {
                    "range": {
                        // | 0 | 1 | 2 | 3 | 4 | 5 | 6 |
                        // | v |   | = |   | ' | ' |
                        "start": { "line": 6, "character": 4 },
                        "end": { "line": 6, "character": 6 }
                    },
                    "newText": "'second'"
                }
            }
        ],
        "isIncomplete": true
    }
}

On accept, PyCharm sent this request, telling that it added second' (with no leading quote but one trailing) at (the position right before) character 5, leaving the trailing quote intact:

{
    "method": "textDocument/didChange",
    "params": {
        "contentChanges": [
            {
                "range": {
                    // | 0 | 1 | 2 | 3 | 4 | 5 | 6 |
                    // | v |   | = |   | ' | ' |
                    // =====================================================
                    // | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | 0 | 1 | 2 |
                    // | v |   | = |   | ' | s | e | c | o | n | d | ' | ' |
                    "start": { "line": 6, "character": 5 },
                    "end": { "line": 6, "character": 5 }
                },
                "text": "second'"
            }
        ]
    }
}

I'll see what I can do.

@InSyncWithFoo InSyncWithFoo changed the title Literal autocomplete Literal autocompletion inserts an extra quote May 9, 2024
@InSyncWithFoo InSyncWithFoo added the enhancement New feature or request label May 9, 2024
@InSyncWithFoo InSyncWithFoo changed the title Literal autocompletion inserts an extra quote Literal autocompletion inserts an extra quote May 9, 2024
@InSyncWithFoo InSyncWithFoo removed the bug Something isn't working label May 18, 2024
@InSyncWithFoo
Copy link
Owner

InSyncWithFoo commented May 24, 2024

Progress update: I'm stuck. The best I can do for now is monkeypatching.

Setup:

from typing import Literal, TypedDict

class D(TypedDict):
	first: int
	second: str

v: Literal['first', 'second']
d: D
v = '|'
v = '''|'''
d = {'|': ''}
d = {'''|''': ''}

Default behavior:

v = 'first'|'
v = '''first'|'''
d = {'second'|': ''}
d = {'''second'|''': ''}

Desired behaviour:

v = 'first'|
v = '''first'''|
d = {'second'|: ''}
d = {'''second'''|: ''}

Monkeypatched behaviour, version 1 (remove the trailing quote for this one case, fallback to default):

v = 'first|'
v = '''first|'''
d = {'second|': ''}
d = {'''second|''': ''}

Monkeypatched behaviour, version 2 (override default entirely: replace the range with replacement, then put caret at the end of replacement):

v = 'first'|
v = '''first'|''
d = {'second'|: ''}
d = {'''second'|'': ''}

Both are nowhere near my expectations. Regardless, I'm pushing them for review.

@InSyncWithFoo
Copy link
Owner

InSyncWithFoo commented May 24, 2024

It turns out PyCharm's behaviour is the same as monkeypatch 1 (no completions for Literals):

d = {'second|': ''}
d = {'''second|''': ''}

...and VSCode's is the same as monkeypatch 2:

v = 'first'|
v = '''first'|''
d = {'second'|: ''}
d = {'''second'|'': ''}

Still, it doesn't change the fact that I consider both of these subpar. This calls for an upstream issue.

@johannesschaffer
Copy link
Author

I would still appreciate if you could align it with pycharms behavior for now. The Jet brains teams takes ages to fix something

@InSyncWithFoo
Copy link
Owner

InSyncWithFoo commented May 24, 2024

@johannesschaffer #52 it is, then. Could you install the fix and play with it for some time first? I already did some testing on my own, but natural workflows tend to be better bug finders.

@johannesschaffer
Copy link
Author

johannesschaffer commented May 24, 2024

Sorry, I don't know how to install a plugin other than via the marketplace

@InSyncWithFoo
Copy link
Owner

See the README. The workflow to choose is the one linked from the PR.

@InSyncWithFoo InSyncWithFoo added the awaiting response Further information is requested label May 27, 2024
@InSyncWithFoo InSyncWithFoo removed the awaiting response Further information is requested label Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment