After commit 2880aae, SignatureInformation.activeParameter
does not have omitempty tag and always marshal. The problem is that its never assigned to anything and always 0
, which overrides actual value in SignatureHelp.activeParameter
, as stated in spec.
Solution is have same value as in SignatureHelp.activeParameter
in SignatureInformation.activeParameter
. The fix is minimal—just a one-line change —and I've submitted a PR.
Comment From: gopherbot
Change https://go.dev/cl/683135 mentions this issue: gopls: fix SignatureHelp activeParameter not being passed
Comment From: jakebailey
Having the parameter count in the signature info itself is good, but IIRC that is an optional feature the client may not use, and so having it set at the top-level reponse is also recommended as a fallback. Perhaps this field should be something else to distinguish "not present" from zero?
Comment From: jakebailey
Eh, maybe it doesn't matter in practice since they'll be the same value (no overloads in Go), and therefore clients will always have the right answer.
Comment From: findleyr
@adonovan reverted this behavior in https://go.dev/cl/677936 for SignatureHelp.ActiveParameter
. I think it was just an oversight not to do the same for SignatureInformation
.
This is an optional field where 0 does not mean 'empty'. It should be a *uint32
and omitempty
.
Comment From: skewb1k
Yes, it seems correct that the field should be optional according to the spec. However, I'm still not fully sure whether it should be nil value. It's unclear how SignatureHelp.activeParameter
could be known while the corresponding activeParameter
of the activeSignature isn't. Maybe if there's always exactly one signature, then the global activeParameter
is enough and there's no need to duplicate it inside the array. Still, it feels a bit odd that the presence of a field in an array element depends on the array's length.
That said, with nil value it works and doesn't conflict with the spec, so I think it's fine.
Comment From: dstpierre
I was going to report this, as a screen reader user this has a decent impact since the screen reader never announce any arguments other than the first one, which isn't ideal ;). Thank you for the efforts.