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

Allow merging two matchit Routers #57

Closed
jplatte opened this issue Sep 7, 2024 · 4 comments · Fixed by #62
Closed

Allow merging two matchit Routers #57

jplatte opened this issue Sep 7, 2024 · 4 comments · Fixed by #62
Labels
feature New feature or request

Comments

@jplatte
Copy link

jplatte commented Sep 7, 2024

axum currently does a bunch of extra bookkeeping of its own in order to be able to merge two of its own Routers. I think some of this could be simplified if there was some functionality in matchit to merge two matchit::Routers. Would you consider a PR for this?

@ibraheemdev
Copy link
Owner

That seems reasonable, I would accept a PR for this.

@ibraheemdev ibraheemdev added the feature New feature or request label Sep 7, 2024
@jplatte
Copy link
Author

jplatte commented Oct 5, 2024

Any implementation hints? ^^
I took a brief look at this, but it doesn't seem straight-forward without understanding the internals of matchit's data structure(s). Is there a document going into that anywhere?

@ibraheemdev
Copy link
Owner

ibraheemdev commented Oct 10, 2024

matchit is a standard prefix trie internally. I think the simplest way to implement this is to create an internal iterator and call insert for every route. A true efficient merge is a little more complex as you have to handle splitting prefixes recursively.

An iterator would involve performing depth/breadth-first search on the tree, collecting the prefix of each node to construct a given route. There's an existing example of reconstructing a specific route here. It may be easier to structure this as a for_each method instead of an iterator due to the recursive search.

@ibraheemdev
Copy link
Owner

Released in 0.8.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants