axum: Update matchit to 0.8.6 and support capture prefixes and suffixes#3143
Draft
axum: Update matchit to 0.8.6 and support capture prefixes and suffixes#3143
Conversation
b22f6c1 to
30ee2c7
Compare
30ee2c7 to
61eab75
Compare
Collaborator
Author
|
I've turned this into a draft because we might wait for |
|
bumping this since matchit 0.9.0 has been released and I am currently depending on a private fork to use it. Would be nice to see it implemented :) |
Member
|
Hi, @mladedav, any update on this PR? May I take over this task if you are not working on it. I also need the newer version of |
Collaborator
Author
|
Sure go for it, I forgot to come back to this after the 0.9 matchit release. I think it should work the same way so it should be pretty straightforward to finish this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Closes #3140
matchitreleased a new version which now supports prefixes and suffixes in routes like\{image}.png.Solution
This updates the dependency and makes sure the stripping works properly with the new routes.
Open Questions
matchitversion to0.8.0(and then later to0.8.4) because changes like these could introduce problems such as that it would be legal to register paths using new matchit features butaxumwould be broken because it wouldn't know how to work with it. We can continue in the same spirit, locking the version to0.8.6or we can relax the requirement to not require the same version. I suspect e.g. this issue could have similar effects.\foodoes not match\foo{capture}might be surprising to users. I've added it to docs about captures but I feel it can be easily overlooked. It is consistent with normal captures which must not be empty either. We could automatically register\fooalong with\foo{capture}but I'm not sure about that.nestandnest_servicewill be subtly different. In shortnestshould be preferred as it will take into consideration the whole path whilenest_servicewill decide which service to call just based on the prefix. More details can be found here. We might consider not allowing partial captures innest_serviceprefixes. I wouldn't expect people to run into this but if someone does, the difference might be very surprising.Pathcan be confusing when using e.g.\{file}.pngas thePath<String>extractor will contain only the capture. I don't think we want to change this but I wanted to flag it if anyone thinks we should.