Skip to content

Support all the registered HTTP methods in the Method enum #332

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

Merged
merged 1 commit into from
Feb 23, 2021
Merged

Support all the registered HTTP methods in the Method enum #332

merged 1 commit into from
Feb 23, 2021

Conversation

zenekron
Copy link
Contributor

@zenekron zenekron commented Feb 6, 2021

This PR extends the Method enum to add all the missing methods from IANA's HTTP Method Registry.
See the issue #331 .

Changes:

  • added missing HTTP methods
  • updated AsRef<str>, FromStr and Method::is_safe accordingly
  • modified Display to leverage AsRef<str>
  • added documentation and links to the appropriate RFC sections
  • added a non-exhausive test that helps catch typos

@jbr
Copy link
Member

jbr commented Feb 9, 2021

Out of curiosity, what is the use case for having WebDAV extensions in http-types?

@zenekron
Copy link
Contributor Author

zenekron commented Feb 9, 2021

Out of curiosity, what is the use case for having WebDAV extensions in http-types?

Besides being able to speak WebDAV itself, there are other tools that reuse subsets of these methods for other purposes.

In my case specifically: I'm porting a small Terraform HTTP backend I wrote from warp to tide, but in order to implement state locking I need to be able to handle LOCK and UNLOCK requests (link).

@jbr
Copy link
Member

jbr commented Feb 9, 2021

Does this mean you'd also want affordances in tide's router for these methods?

@zenekron
Copy link
Contributor Author

Does this mean you'd also want affordances in tide's router for these methods?

I'm not too sure what you mean by that (not a native English speaker here), but if you're talking about creating methods like tide::Route::get and tide::Route::post that wrap around tide::Route::method, then I think these HTTP methods are uncommon enough that doing so would not bring much benefit.

That being said if you think it's appropriate I can open a PR against tide as well, just let me know.

@Fishrock123
Copy link
Member

if you're talking about creating methods like tide::Route::get and tide::Route::post that wrap around tide::Route::method

That is indeed what @jbr meant.

@yoshuawuyts
Copy link
Member

I think these HTTP methods are uncommon enough that doing so would not bring much benefit.

I agree with this rationale; I don't think we need to expose shorthands for them, but having them as part of the enum seems reasonable enough.

Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing; thank you!

@yoshuawuyts yoshuawuyts merged commit a47a9b1 into http-rs:main Feb 23, 2021
@Fishrock123 Fishrock123 mentioned this pull request Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants