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

Add URL Scheme parser and test #13

Merged
merged 1 commit into from
May 20, 2022
Merged

Add URL Scheme parser and test #13

merged 1 commit into from
May 20, 2022

Conversation

ryanbooker
Copy link
Contributor

@ryanbooker ryanbooker commented May 12, 2022

πŸ’¬ Summary of Changes

  • Implements a scheme parser to support routing older style custom schemes on iOS.

βœ… Checklist

  • 🧐 Performed a self-review of the changes.
  • βœ… Added tests to cover changes.
  • 🏎 Ran Unit Tests before opening PR.

@stephencelis
Copy link
Member

@ryanbooker Thanks for taking the time to PR! There was a similar one opened here before we extracted the library that was closed in favor of the baseURL and baseRequestData methods. Curious if you agree with this or if you have some example uses of where parsing the scheme can be useful?

I think generally the most important parsers are the ones that can change throughout a single server process, and that nginx/Apache are kind of responsible for the server-level things, like scheme, host, etc.

@robfeldmann
Copy link
Contributor

Curious if you agree with this or if you have some example uses of where parsing the scheme can be useful?

I've been thinking about this too. I think being able to parse any part of the URL components is useful. For example, I have a need to parse/match the fragment of a URL for deep linking purposes, so I'm working on a Fragment ParserPrinter (although it's taking me an embarrassing amount of time to get it right).

I would also like to add a Host parser for the same reason. I can see needing to be able match on a custom scheme as well, even if it isn't suitable or recommended for printing/routing.

@ryanbooker
Copy link
Contributor Author

ryanbooker commented May 12, 2022

Thanks for the response @stephencelis. I'm not sure I quite follow, and I've only been using the Library for "5 min" so if there's another way to do what I want, please point me at it! πŸ˜„

I want to parse a specific custom scheme on the app side (ignoring the fact that Apple discourages using them nowadays).

I'm working with some legacy code where the schemes can't be changed at the moment, and there are several custom schemes that have been used as routes. So rather than customScheme://route1, customScheme://route2, I have route1:// and route2://.

I want to parse a specific scheme and the parser should fail if the scheme doesn't match…

I can keep the Scheme parser in my local code, but thought it may be useful for others. But if there's a better way to do what I'd like, please let me know. πŸ˜…

UPDATE: Turns out I misread the current code, and there is only one Scheme, however all the routes are based on the Host rather than a Path. e.g.customerSceheme://route1 and customScheme://route2. I can add a similar Host parser if you decide either of these are useful/nice to have. πŸ™‚

@stephencelis
Copy link
Member

UPDATE: Turns out I misread the current code, and there is only one Scheme, however all the routes are based on the Host rather than a Path. e.g.customerSceheme://route1 and customScheme://route2. I can add a similar Host parser if you decide either of these are useful/nice to have. πŸ™‚

Typically if you want a path you would use customScheme:///route1, etc., but I can see that if it's out of your control a host parser could be useful...I think the thing we want to figure out is accidental misuse, e.g., if you use Host(…) or Scheme(…) because you want them to print in your routes, baseURL is often more appropriate, but docs can probably steer people the right direction.

Because of this, I think we'll take this after all! If you create a Host parser, please do submit a PR for it!

@stephencelis stephencelis merged commit 29b8720 into pointfreeco:main May 20, 2022
@ryanbooker
Copy link
Contributor Author

Cool. No worries. I'll make a Host version as well. Thanks. :) customScheme:///route1 makes sense, but the urls aren't really under my control, at least in the short term. Cheers. :)

@ryanbooker ryanbooker deleted the feature/add_scheme_parser branch June 11, 2022 13:46
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.

None yet

3 participants