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

Headers should offer method to remove and return header #891

Closed
untitaker opened this issue Aug 19, 2016 · 4 comments
Closed

Headers should offer method to remove and return header #891

untitaker opened this issue Aug 19, 2016 · 4 comments
Labels
A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Milestone
0.11

Comments

@untitaker
Copy link
Contributor

If I take a Headers by value (moving it into my function), I want to avoid copying them. For that reason it would be useful if either Headers::remove would return the popped item (as HashMap::remove does), or if a new method was added.
@untitaker
Copy link
Contributor Author

(I'm willing to write a PR for this)

@untitaker untitaker mentioned this issue Aug 19, 2016
Merged
@seanmonstar
Copy link
Member

Hm, I agree with this entirely, 👍 . This would be a breaking change, since remove currently returns a bool, but it seems like the right one to make. As such, I'd aim it for master and not for the 0.9.x branch. Basically:

fn remove<H: Header>(&mut self) -> Option<H>;

@seanmonstar seanmonstar added A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor. labels Aug 22, 2016
@seanmonstar seanmonstar added this to the 0.10 milestone Aug 22, 2016
@untitaker
Copy link
Contributor Author

Great, I will write a PR as soon as possible.

untitaker added a commit to untitaker/hyper that referenced this issue Aug 22, 2016
@untitaker
Make Headers::remove return the header
d89a554 
Fix hyperium#891
@untitaker untitaker mentioned this issue Aug 22, 2016
Merged
untitaker added a commit to untitaker/hyper that referenced this issue Aug 22, 2016
@untitaker
feat(headers): Headers::remove returns the header
c389c48 
Fix hyperium#891
@untitaker
Copy link
Contributor Author

@seanmonstar Work in progress at #898, I underestimated the effort and need some guidance about further procedure.

untitaker added a commit to untitaker/hyper that referenced this issue Aug 23, 2016
@untitaker
feat(headers): Headers::remove returns the header
2e29a3d 
Fix hyperium#891
untitaker added a commit to untitaker/hyper that referenced this issue Aug 23, 2016
@untitaker
feat(headers): Headers::remove returns the header
bc97d7b 
Fix hyperium#891
untitaker added a commit to untitaker/hyper that referenced this issue Aug 23, 2016
@untitaker
feat(headers): Headers::remove returns the header
81e3cef 
Fix hyperium#891
seanmonstar pushed a commit that referenced this issue Aug 23, 2016
@untitaker @seanmonstar
feat(headers): Headers::remove returns the Header
9375add 
Closes #891

BREAKING CHANGE: `Headers.remove()` used to return a `bool`,
  it now returns `Option<H>`. To determine if a a header exists,
  switch to `Headers.has()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-headers Area: headers. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Milestone
0.11
Development

No branches or pull requests
2 participants
@seanmonstar @untitaker