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

Don't extend selection when completion is triggered by typing one or more trigger characters #78

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tintoy
Copy link
Owner

@tintoy tintoy commented Oct 15, 2023

This is because VS Code behaviour has changed, and it no longer correctly handles extension of selection if it has inserted an auto-closing delimiter.

#71

…preting the target location and its surrounding XML

#71
tintoy/msbuild-project-tools-server#129
…more trigger characters

This is because VS Code behaviour has changed, and it no longer correctly handles extension of selection if it has inserted an auto-closing delimiter.

#71
@tintoy
Copy link
Owner Author

tintoy commented Oct 15, 2023

Note: I haven't removed HandleTriggerCharacters because of how long it took me to troubleshoot this only to discover that it was partially due to lost knowledge that it was so hard to diagnose.

Copy link
Collaborator

@DoctorKrolic DoctorKrolic left a comment

Choose a reason for hiding this comment

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

Unfortunatelly, this solution doesn't work for me. It breaks existing << problem, bud doesn't fix the problem:
Code_R0h50bxJy4

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

Bummer - I’ll have to see whether we can be more selective then. That other completion makes no sense; the rest of the completion text is missing.

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

I still can’t figure out what’s going on but it seems to be at the intersection of the language server, protocol-level LSP changes and VSCode’s behaviour for auto-closing angle brackets.

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

Actually, I'm having trouble reproducing your issue:

correct-completion-behaviour

Could it be something to do with editor settings?

Here are mine:

{
    "workbench.startupEditor": "none",
    "git.inputValidationLength": 300,
    "git.inputValidationSubjectLength": null,
    "git.confirmSync": false,
    "git.autofetch": true,
    "workbench.editor.untitled.hint": "hidden",
    "[jsonc]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },
    "explorer.confirmDragAndDrop": false,
    "security.workspace.trust.untrustedFiles": "open",
    "[json]": {
        "editor.defaultFormatter": "vscode.json-language-features"
    },
    "explorer.confirmDelete": false,
    "git.enableSmartCommit": true,
    "explorer.fileNesting.enabled": true,
    "explorer.fileNesting.expand": false,
    "redhat.telemetry.enabled": false,
    "[typescript]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },
    "[html]": {
        "editor.defaultFormatter": "esbenp.prettier-vscode"
    },
    "prettier.printWidth": 200,
    "[python]": {
        "editor.formatOnType": true
    },
    "vs-kubernetes": {
        "vscode-kubernetes.helm-path.linux": "/home/tintoy/.local/state/vs-kubernetes/tools/helm/linux-amd64/helm",
        "vscode-kubernetes.minikube-path.linux": "/home/tintoy/.local/state/vs-kubernetes/tools/minikube/linux-amd64/minikube"
    },
    "security.workspace.trust.banner": "always",
    "[msbuild]": {
        "editor.quickSuggestions": {
            "other": "off"
        }
    },
    "editor.renderWhitespace": "all",
    "files.associations": {
        "*.csproj": "msbuild"
    },
    "dotnetAcquisitionExtension.existingDotnetPath": [
        {
            "extensionId": "tintoy.msbuild-project-tools",
            "path": "C:\\Program Files\\dotnet\\dotnet.exe"
        },
        {
            "extensionId": "ms-dotnettools.csharp",
            "path": "C:\\Program Files\\dotnet\\dotnet.exe"
        }
    ],
    "workbench.editor.empty.hint": "hidden",
    "msbuildProjectTools.logging.level": "Verbose",
    "msbuildProjectTools.logging.trace": true
}

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

Got it! Can you change the language type from XML to MSBuild?

It looks like something may have changed in the VSCode-provided language definition for XML (which our MSBuild language definition is not affected by).

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

VSCode-provided XML language definitions changed sometime last year, it seems:

https://github.com/microsoft/vscode/commits/1da76fe8ff9c7d10f9161df2f783cbea03f8664d/extensions/xml

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

Maybe this one?

microsoft/vscode@f572583

@tintoy
Copy link
Owner Author

tintoy commented Oct 21, 2023

Actually, it looks like the XML language configuration is significantly different to our MSBuild language configuration, now (even ignoring minor syntax changes in the language config):

Ours: https://github.com/tintoy/msbuild-project-tools-vscode/blob/master/language-configuration.json
Theirs: https://github.com/microsoft/vscode/blob/main/extensions/xml/xml.language-configuration.json

For one thing, < is not listed as an auto-closing tag in the XML language definition but it is, in the MSBuild language definition.

@DoctorKrolic
Copy link
Collaborator

Can you change the language type from XML to MSBuild?

That is really really bad. We should provide equivalent features both to xml and msbuild languages. I would say, xml is even more importand since it is a well-known language and other extension, that provide general-purpose features like formatting for instance, target xml. Especially considering that completions is one of primary features of our extension, so it should work properly. Therefore I don't think we can merge this until the problem is resolved for both language definitions

@tintoy
Copy link
Owner Author

tintoy commented Oct 22, 2023

I’m open to suggestions if you have any ideas as to how we can do that but I’m out of ideas I’m afraid. The way the XML language is defined now just doesn’t work with how we do completions (and we can’t control the XML language - it’s a shared definition).

To be clear, the completions we return are valid and legal completions; it's just that VS Code is not behaving consistently with them. And given how language configurations work, it's starting to feel a little too much like magic ritual for my tastes:

https://code.visualstudio.com/api/language-extensions/language-configuration-guide

@tintoy
Copy link
Owner Author

tintoy commented Oct 22, 2023

And due to the XML parser we use, we need the closing tag (>) to be inserted when the opening tag (<) is inserted, or we don't have a valid location in the XML (an element with no name - <> is still an element; without the closing > it's just too ambiguous as to which element we're working with).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants