Skip to content

Commit characters broken in simple cases #181567

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

Closed
duzenko opened this issue May 3, 2023 · 10 comments
Closed

Commit characters broken in simple cases #181567

duzenko opened this issue May 3, 2023 · 10 comments
Assignees
Labels
*duplicate Issue identified as a duplicate of another issue(s) javascript JavaScript support issues typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code)

Comments

@duzenko
Copy link

duzenko commented May 3, 2023

Bug Report

πŸ”Ž Search Terms

vscode complete dot
vscode commit characters
typescript commit characters

πŸ•— Version & Regression Information

Most likely never worked properly

⏯ Playground Link

[<!--
A link to a TypeScript Playground "Share" link which shows this behavior

The TypeScript Workbench can be used for more complex setups, try
https://www.typescriptlang.org/dev/bug-workbench/

As a last resort, you can link to a repo, but these will be slower for us to investigate.
-->
Playground link with relevant code](https://www.typescriptlang.org/play?#code/PTAEHUFMBsGMHsC2lQBd5oBYoCoE8AHSAZVgCcBLA1UABWgEM8BzM+AVwDsATAGiwoBnUENANQAd0gAjQRVSQAUCEmYKsTKGYUAbpGF4OY0BoadYKdJMoL+gzAzIoz3UNEiPOofEVKVqAHSKymAAmkYI7NCuqGqcANag8ABmIjQUXrFOKBJMggBcISGgoAC0oACCbvCwDKgU8JkY7p7ehCTkVDQS2E6gnPCxGcwmZqDSTgzxxWWVoASMFmgYkAAeRJTInN3ymj4d-jSCeNsMq-wuoPaOltigAKoASgAywhK7SbGQZIIz5VWCFzSeCrZagNYbChbHaxUDcCjJZLfSDbExIAgUdxkUBIursJzCFJtXydajBBCcQQ0MwAUVWDEQC0gADVHBQGNJ3KAALygABEAAkYNAMOB4GRonzFBTBPB3AERcwABS0+mM9ysygc9wASkUAHEGMEVDhmh4yF5EOLnMD2DQvm4zMx2AxmJB+LBoOpEhz4HoRF4+XSGUzBHykti+eAHKgAOTCABykAkfKCKgA8l8yO9BO6tJAjqhHApXNI8KAnFadMMsPoUAhENDhJcHRJxdERAYONiFkxWBweEESkA)

πŸ’» Code

const anExampleVariable = "Hello World"
console.log(anExampleVariable)
Ga<cursor>

πŸ™ Actual behavior

Typing Ga, then dot puts dot after Ga

πŸ™‚ Expected behavior

Whatever suggestion is selected (e.g. GainNode) must be completed and inserted in the code, followed by a dot

@duzenko
Copy link
Author

duzenko commented May 3, 2023

This particular code snippet works in VSCode, but not in TypeScript Workbench
However more complex cases fail in VSCode as well, e.g.

export function drawBackground() {
    const backgroundImage = ga<cursor> getImageByName('field/green-terrain-v2.jpg')
    context.fillStyle = "black"
    context.fillRect(0, 0, canvas.width, canvas.height)
    if (backgroundImage) {
        const p = cellToScreen(new GridCell(arena.columns.first(), arena.rows.first()))
        const w = (canvas.width / 2 - p.x) * 1.6
        const h = (p.y - canvas.height / 2) * 1.25
        context.drawImage(backgroundImage, canvas.width / 2 - w, canvas.height / 2 - h, 2 * w, 2 * h)
    }
}

Please just always accept dot to complete, whatever flawed context-based logic you're trying to follow here is only adding frustration for everyone

@RyanCavanaugh RyanCavanaugh transferred this issue from microsoft/TypeScript May 4, 2023
@duzenko
Copy link
Author

duzenko commented May 5, 2023

@RyanCavanaugh Why did you move the issue? Don't you know that similar issues have been auto-closed in the vscode tracker?

Unfortunately we can't reliably compute commit characters on the VS Code extension side. This needs to come from the TypeScript server. Support for commit characters there is tracked by microsoft/TypeScript#27623

#110382 (comment)

@mjbvz please move the issue back to the typescript tracker instead of closing it like you did with other issues

@mjbvz
Copy link
Collaborator

mjbvz commented May 10, 2023

Works fine for me in VS Code. Is this only an issue in the TS playground?

@mjbvz mjbvz added the info-needed Issue requires more information from poster label May 10, 2023
@duzenko
Copy link
Author

duzenko commented May 10, 2023

@mjbvz
There are two snippets here: one fails in the playground, and the other is the real world example that fails in VSCode

@mjbvz
Copy link
Collaborator

mjbvz commented May 10, 2023

Minimal example:

function foo() {
    const x = fo // trigger suggest and type . here
}

@RyanCavanaugh I still think TS needs to be the one computing the commit characters. This will align it more with the LSP and let editors have a consistent UX

@vscodenpa
Copy link

This issue has been closed automatically because it needs more information and has not had recent activity. See also our issue reporting guidelines.

Happy Coding!

@vscodenpa vscodenpa closed this as not planned Won't fix, can't repro, duplicate, stale May 18, 2023
@duzenko
Copy link
Author

duzenko commented May 18, 2023

@mjbvz @RyanCavanaugh what info is needed exactly here?

@RyanCavanaugh
Copy link
Member

@mjbvz is investigating what the next steps should be here in terms of whether TS server or VS code needs to change its behavior and how. We talked about this at the typescript vs code sync yesterday and everyone was in agreement that, ideally, dot is indeed a completion character here, we just need to work out the details of whether that's always the case and if not, how we figure out when it is.

@mjbvz mjbvz reopened this May 18, 2023
@mjbvz mjbvz added this to the May 2023 milestone May 18, 2023
@mjbvz
Copy link
Collaborator

mjbvz commented May 18, 2023

@RyanCavanaugh I investigated this further. We disable commit characters in this case because TypeScript returns isNewIdentifierLocation on the completionInfo response. This issue is actually already being tracked by microsoft/TypeScript#31244

Closing this as a duplicate of microsoft/TypeScript#31244. I still think it would make sense to have TS compute the commit characters instead of VS Code. For reference, here's where we compute these today:

private static getCommitCharacters(context: CompletionContext, entry: Proto.CompletionEntry): string[] | undefined {

@mjbvz mjbvz closed this as completed May 18, 2023
@mjbvz mjbvz added the *duplicate Issue identified as a duplicate of another issue(s) label May 18, 2023
@mjbvz mjbvz removed this from the May 2023 milestone May 18, 2023
@mjbvz mjbvz added upstream Issue identified as 'upstream' component related (exists outside of VS Code) typescript Typescript support issues javascript JavaScript support issues and removed info-needed Issue requires more information from poster labels May 18, 2023
@duzenko

This comment was marked as spam.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*duplicate Issue identified as a duplicate of another issue(s) javascript JavaScript support issues typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
Development

No branches or pull requests

4 participants