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

Jump to bottom

x/tools/gopls: add support for provided WorkDoneToken in ExecuteCommand #40527

Closed
leitzler opened this issue Aug 1, 2020 · 5 comments
Closed

x/tools/gopls: add support for provided WorkDoneToken in ExecuteCommand #40527

leitzler opened this issue Aug 1, 2020 · 5 comments
Labels
FrozenDueToAge 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/v0.5.0

Comments

@leitzler
Copy link
Contributor

leitzler commented Aug 1, 2020

What version of Go are you using (go version)?

$ go version
-
x/tools@644d4167

What did you do?

Called ExecuteCommand with a provided WorkDoneToken:

&protocol.ExecuteCommandParams{
     Command:   "generate",
     Arguments: {
         {0x22, 0x66, 0x69, 0x6c, 0x65, 0x3a, 0x2f, 0x2f, 0x2f, 0x70, 0x72, 0x69, 0x76, 0x61, 0x74, 0x65, 0x2f, 0x76, 0x61, 0x72, 0x2f, 0x66, 0x6f, 0x6c, 0x64, 0x65, 0x72, 0x73, 0x2f, 0x78, 0x6d, 0x2f, 0x6c, 0x73, 0x32, 0x30, 0x38, 0x78, 0x64, 0x31, 0x37, 0x34, 0x76, 0x39, 0x35, 0x70, 0x67, 0x64, 0x32, 0x30, 0x72, 0x6e, 0x5f, 0x71, 0x62, 0x68, 0x30, 0x30, 0x30, 0x30, 0x67, 0x6e, 0x2f, 0x54, 0x2f, 0x74, 0x6d, 0x70, 0x2e, 0x5a, 0x53, 0x69, 0x38, 0x48, 0x67, 0x49, 0x47, 0x22},
         {0x66, 0x61, 0x6c, 0x73, 0x65},
     },
     WorkDoneProgressParams: protocol.WorkDoneProgressParams{
         WorkDoneToken: int(1001),
     },
}

When gopls starts to execute go generate, it sends a ShowMessageRequest to the client so the client can present a dialog to the user. That dialog includes an action, Cancel, to let the user cancel go generate before it's done.

Using a WorkDoneToken in the ExecuteCommand request is the only way I found to tie the dialog to a specific execution. If it can't be tied to that specific execution there is no way to know when the dialog should be automatically closed (i.e. when go generate is done and Cancel isn't a valid action anymore).

What did you expect to see?

The WorkDoneToken used during progress reporting.

What did you see instead?

A new token created for me:

WorkDoneProgressCreate callback: &protocol.WorkDoneProgressCreateParams{
    Token: "8674665223082153551",
}

That is used to report progress:

Progress callback: &protocol.ProgressParams{
    Token: "8674665223082153551",
    Value: map[string]interface {}{
        "cancellable": bool(true),
        "kind":        "begin",
        "message":     "running go generate",
        "title":       "generate",
    },
}
Progress callback: &protocol.ProgressParams{
    Token: "8674665223082153551",
    Value: map[string]interface {}{
        "cancellable": bool(true),
        "kind":        "report",
        "message":     "sleep 10\n",
    },
}
Progress callback: &protocol.ProgressParams{
    Token: "8674665223082153551",
    Value: map[string]interface {}{
        "kind":    "end",
        "message": "finished",
    },
}
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Aug 1, 2020
@gopherbot gopherbot added this to the Unreleased milestone Aug 1, 2020
@leitzler
Copy link
Contributor Author

leitzler commented Aug 2, 2020

ShowMessageRequest isn't sent when WorkDoneProgress is supported by the client, so this isn't as urgent as I initially thought. The entire lifetime can be traced using progress callbacks only, even though they can't be linked to a specific command execution.

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Aug 4, 2020
@stamblerre
Copy link
Contributor

I don't think we actually ever use the WorkProgressToken passed in by any request, though we should. @findleyr has the most experience with this code, and maybe have some thoughts here?

@findleyr
Copy link
Contributor

findleyr commented Aug 7, 2020

Yeah, this is a bug. I'll fix it.

@findleyr findleyr self-assigned this Aug 7, 2020
@gopherbot
Copy link

Change https://golang.org/cl/247321 mentions this issue: internal/lsp: use the token supplied by the client for progress

@gopherbot
Copy link

Change https://golang.org/cl/247407 mentions this issue: internal/lsp/progress: refactor progress reporting

@stamblerre stamblerre mentioned this issue Aug 8, 2020
Closed
gopherbot pushed a commit to golang/tools that referenced this issue Aug 10, 2020
@findleyr
internal/lsp: use the token supplied by the client for progress
30a0893 
Our WorkDone reporting was generating a random token for each unit of
work, even if a token was supplied by the client.  Change this to use
the client token if it is non-empty, and skip the
workDoneProgress/create request.

After this change we can no longer rely on tokens being a string.
Update our progress tracking accordingly.

For golang/go#40527

Change-Id: I702f739c466efb613b69303aaf07005addd3b5e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247321
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre stamblerre added this to the gopls/v.0.4.5 milestone Aug 10, 2020
@golang golang locked and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge 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/v0.5.0
Development

No branches or pull requests
4 participants
@leitzler @stamblerre @gopherbot @findleyr