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.

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

x/tools/gopls: gopls does not reset workspace/configuration if missing from the client #71227

Open
h9jiang opened this issue Jan 10, 2025 · 6 comments
Open

x/tools/gopls: gopls does not reset workspace/configuration if missing from the client #71227

h9jiang opened this issue Jan 10, 2025 · 6 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/backlog

Comments

@h9jiang
Copy link
Member

h9jiang commented Jan 10, 2025

gopls version

master head, following experiment is based on commit 6efe0f4b404b25e02999c3e34db08771f855fc28

go env

NA

What did you do?

Locate semantic token information from gopls of a string or a number. Toggle feature noSemanticString or noSemanticNumber.

What did you see happen?

By default, number's token info is returned from the gopls, you can see semantic token type = number.

image

Then if I set "ui.noSemanticNumber": true, the semantic token info goes away. WAI

image

If I set the "ui.noSemanticNumber": false,, the semantic token info comes back. WAI

image

Set it to "ui.noSemanticNumber": true,, the token info goes away. WAI. Then if I comment this out. What I expect is, the semantic token information will come back. Because, no setting means default value "ui.noSemanticNumber": false,.

What did you expect to see?

image

As a result it does not come back.

Editor and settings

No response

Logs

I will focus on the last two workspace/configuration logs

Where I set it to true

[Trace - 5:26:49 PM] Sending response 'workspace/configuration - (14)'. Processing request took 0ms
Result: [
    {
        "ui.semanticTokens": true,
        "ui.noSemanticNumber": true,
        "ui.inlayhint.hints": {
            "assignVariableTypes": false,
            "compositeLiteralFields": false,
            "compositeLiteralTypes": false,
            "constantValues": false,
            "functionTypeParameters": false,
            "parameterNames": false,
            "rangeVariableTypes": false
        },
        "ui.vulncheck": "Off",
        "linkifyShowMessage": true
    }
]

Where I comment it out

[Trace - 5:27:44 PM] Sending response 'workspace/configuration - (16)'. Processing request took 0ms
Result: [
    {
        "ui.semanticTokens": true,
        "ui.inlayhint.hints": {
            "assignVariableTypes": false,
            "compositeLiteralFields": false,
            "compositeLiteralTypes": false,
            "constantValues": false,
            "functionTypeParameters": false,
            "parameterNames": false,
            "rangeVariableTypes": false
        },
        "ui.vulncheck": "Off",
        "linkifyShowMessage": true
    }
]

I think what happened is, when gopls saw "ui.noSemanticNumber": X, in the json, gopls will apply this X in memory. WAI.
But when gopls saw "ui.noSemanticNumber": X, is missing in the json, gopls do nothing. In this case, gopls should set the ui.noSemanticNumber = false because false is the default value.

@findleyr
@h9jiang h9jiang added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 10, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 10, 2025
@h9jiang h9jiang mentioned this issue Jan 10, 2025
Closed
@gabyhelp
Copy link

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 10, 2025
@findleyr
Copy link
Member

I agree this is a bug, but perhaps not one that is easy to fix.

The problem is that on didChangeConfiguration notifications, gopls makes a workspace/configuration request to the client with a specific scope URI set to the workspace folder. IIUC, the VS Code client will respond with the workspace settings.json.

Has that response already merged user settings.json? Or was user settings.json sent as the initializationOptions?

If the former, we should always clobber the existing options with defaults, and then apply the result of workspace/configuration. If the latter, we need to keep a copy of the original initializationOptions that can be overlaid with workspace options.

Also: in our workspace/configuration request we ask for the "gopls" section of configuration, but are tolerant of users sending us configuration without the "gopls." prefix. Therefore, some clients will send an empty response to "workspace/configuration", and so only work if we overlay the "initializationOptions" of the original "initialize" request.

In summary: something we're doing is certainly wrong, but any change to our current behavior is liable to be complicated, and break some existing users. Therefore, we are unlikely to be able to solve this problem soon.

@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Jan 15, 2025
@the1337beauty
Copy link

I have no idea if this is related, but in my VSCode settings when I set ui.semanticTokenTypes for string to false, it has no effect, but I believe it's supposed to replicate the effects I'm seeing for ui.noSemanticString: true.

 "gopls": {
    "ui.semanticTokens": true,
    "ui.noSemanticString": true, // this works but gives notification that it's deprecated
    "ui.semanticTokenTypes": {
      "string": false, // this does not work for %s/%v type directives or escaped characters like \n
    }

@h9jiang
Copy link
Member Author

h9jiang commented Mar 10, 2025 •
edited
Loading

It could be related to #71964 issue. Could you try upgrading your gopls to v0.18.1, we fixed it in v0.18.1.

If you verify the v0.18.1 fix the issue, you can remove the ui.noSemanticString and ui.semanticTokens because they are being deprecated. We still honoring those configuration for 0.18 but may not honoring them in the future.

What you mentioned might not related to this issue. Explicitly setting make sure gopls will always honor your configuration. But if you comment it out or delete it from your configuration, gopls might not aware of your change.

@the1337beauty
Copy link

@h9jiang Confirmed I have v0.18.1 but I'm still having the issue. If you're saying that semanticTokens is also being deprecated, then what is the proper way to handle this in VSCode settings? I was in the process of opening a new issues but didn't want to duplicate things.

@h9jiang
Copy link
Member Author

h9jiang commented Mar 11, 2025 •
edited
Loading

I'm sorry, ui.semanticTokens is not being deprecated. My mistake.

What I meant is noSemanticString and noSemanticNumber are being deprecated. I did not read your configuration careful enough and I just copy the content from your json config. The purpose of semanticTokenTypes is to replace noSemanticString and noSemanticNumber.

I made another mistake on this. #71964 issue will be fixed in gopls v0.18.2. But gopls v0.18.2 is not released yet. The milestone for v0.18.2 is here. Until that is released, I think you might have to keep using noSemanticString and noSemanticNumber. Once v0.18,2 that is released, vscode-go will suggest you to upgrade and you should be able to get rid of noSemanticString and noSemanticNumber replaced by ui.semanticTokenTypes.string = false.

I'm so sorry for this confusion. How could I make two mistakes at the same time. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BugReport Issues describing a possible bug in the Go implementation. gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/backlog
Development

No branches or pull requests
5 participants
@gopherbot @the1337beauty @findleyr @h9jiang @gabyhelp