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

net/url: add Values.Has(key) #45100

Closed
iann0036 opened this issue Mar 18, 2021 · 9 comments
Closed

net/url: add Values.Has(key) #45100

iann0036 opened this issue Mar 18, 2021 · 9 comments

Comments

@iann0036
Copy link
Contributor

iann0036 commented Mar 18, 2021

There is currently no inbuilt method within Values of detecting whether a query parameter without a value is set (e.g. http://example.com/?queryname).

I propose adding the following method to determine whether a specific key is set:

// Checks whether a given key is set.
func (v Values) IsSet(key string) bool {
	if _, ok := v[key]; ok {
		return true
	}
	return false
}

This would also work for query parameters with values.

@gopherbot gopherbot added this to the Proposal milestone Mar 18, 2021
@martisch
Copy link
Contributor

Can this be reduced to?

func (v Values) IsSet(key string) bool {
	_, ok := v[key]
	return ok
}

So instead of writing _, ok := v[key] with the proposal v.IsSet(key) would be able to be used instead?

@iann0036
Copy link
Contributor Author

Yep, that is more concise, I agree.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Mar 19, 2021
@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

It seems like the usual Go word is Has for this (HasPrefix, HasSuffix, types.TypeAndValue.HasOk, etc).
Otherwise it seems fine - we typically don't bother and just use the map directly, but we've added all the other operations as methods on Values already.

@rsc rsc changed the title proposal: net/url: add Values.IsSet() proposal: net/url: add Values.Has(hey) Apr 7, 2021
@rsc rsc changed the title proposal: net/url: add Values.Has(hey) proposal: net/url: add Values.Has(key) Apr 7, 2021
@rsc rsc moved this from Incoming to Active in Proposals (old) Apr 7, 2021
@rsc
Copy link
Contributor

rsc commented Apr 7, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 14, 2021

There are 5 thumbs down on the original, but it does seem like this is the one operation that we haven't put into the Values API, and we might as well.

Does anyone object to adding Values.Has? If so, why?

@cristaloleg
Copy link

@rsc I suppose it's due to naming, Has (as you've mentioned above) is more natural for Go.

@rsc rsc moved this from Active to Likely Accept in Proposals (old) Apr 21, 2021
@rsc
Copy link
Contributor

rsc commented Apr 21, 2021

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 28, 2021

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc moved this from Likely Accept to Accepted in Proposals (old) Apr 28, 2021
@rsc rsc changed the title proposal: net/url: add Values.Has(key) net/url: add Values.Has(key) Apr 28, 2021
@rsc rsc modified the milestones: Proposal, Backlog Apr 28, 2021
iann0036 added a commit to iann0036/go that referenced this issue Apr 28, 2021
Adds a method within Values of detecting whether a query parameter without a value is set.

Fixes golang#45100
@gopherbot
Copy link

Change https://golang.org/cl/314850 mentions this issue: net/url: add Values.Has

@rsc rsc mentioned this issue Jun 10, 2021
83 tasks
@golang golang locked and limited conversation to collaborators Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants