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

Proposal: Add context.NewKey #38559

Closed
Merovius opened this issue Apr 21, 2020 · 3 comments
Closed

Proposal: Add context.NewKey #38559

Merovius opened this issue Apr 21, 2020 · 3 comments

Comments

@Merovius
Copy link
Contributor

I would like to propose adding a new function to the context package. It would have an implementation similar to errors.New, so roughly

package context

type key struct {
    s string
}

func (k *key) String() string {
    return k.s
}

// NewKey returns a new value, formatting as s, suitable as a key for use with WithValue.
// Each call will return a distinct value even if the text is identical.
func NewKey(s string) interface{} {
    return &key{s}
}

Rationale:

The current recommended best-practice for context-Keys is to create a new, unexported type and use a value of that type as a key. This requires boiler-plate, in particular if you want the key to format nicely. With this, creating a key would just be var myCtxKey = context.NewKey("foo"), to provide the same benefits.

It's just a small quality of life improvement. And having a standard, trivially discoverable way to create context-keys also prevents people from accidentally using unsuitable keys. This could, of course, also live outside the stdlib (and it probably already does) but I would imagine that the improvement is small enough to not really be worth searching for and importing an extra package for it (at least it isn't to me).

I fully understand if the benefits are deemed to small, of course. Just thought I'd suggest it :)

@gopherbot gopherbot added this to the Proposal milestone Apr 21, 2020
@ccbrown
Copy link

ccbrown commented Apr 22, 2020

Is formatting context keys common? I've definitely never felt compelled to print a context key and IMO they should be treated as totally opaque.

I think I'm on board with a context.NewKey function since having first class support for creating keys might make it harder for people skimming the documentation to do the wrong thing (implement collision-prone or memory-allocating keys), but I would probably prefer it to just be context.NewKey(). If your key needs more functionality, you can implement it yourself.

@Merovius
Copy link
Contributor Author

@ccbrown fmt formats contexts, if you print them - they implement Stringer. Though while I assumed that it formats the key, TIL, it actually just prints the type's name. Either way, you'd want a way to disambiguate keys returned by NewKey when formatting a context. IMO a string-argument would be fine, but another possibility might be to use NewKey caller's location (maybe package and file/line-no or something) and yet another (more opaque) would be to use the address.

Personally I don't super care about if and how it formats. I just don't want to have to define new types as context keys :)

@Merovius
Copy link
Contributor Author

Actually, I guess with the formatting as it is, the benefit here becomes negligible. Lemme just close this pre-emptively :)

@golang golang locked and limited conversation to collaborators Apr 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants