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

Support named wildcards (PR) #45

Open
chucksellick opened this issue Jan 6, 2018 · 2 comments
Open

Support named wildcards (PR) #45

chucksellick opened this issue Jan 6, 2018 · 2 comments

Comments

@chucksellick
Copy link

Firstly - I have already created a pull request for this feature, I realise the contributing guidelines say not to do this, but in my case I needed the feature anyway and am using that build in my routing library: https://github.com/downplay/jarl-react

So, I will not be too concerned if the pull request is ultimately declined, as I will carry on using my fork anyway, but I am opening this issue as it would still be nice to see the functionality in the main package, and am happy to discuss any changes to make this happen. I am also aware that this feature has been mentioned elsewhere and is probably desirable (but maybe you were thinking of different syntax).

The pull request is here: #44

The syntax I have implemented for named wildcards is: /*:path which will capture everything into path instead of _. I chose this syntax because it is unambiguous compared to existing syntax and did not break any existing tests, and I feel the meaning is instantly clear when reading the syntax.

I've added a complete set of tests for the syntax, BUT I did not update any documentation - if discussion is positive then I am happy to also update the docs and add to the pull request.

I look forward to feedback!

@snd
Copy link
Owner

snd commented Jan 22, 2018

thanks. this enhancement is reasonable and the implementation is well thought out. i'll include it in a new release soon. very busy working on other things atm

@chucksellick
Copy link
Author

I've pushed a documentation update now for the new syntax. Will be great to see this merged.

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

No branches or pull requests

2 participants