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

Change member ownership #14

Merged
merged 33 commits into from
Jun 14, 2024
Merged

Conversation

RokerHRO
Copy link
Collaborator

@RokerHRO RokerHRO commented Dec 5, 2023

I've implemented my ideas für #11 in a separate branch in my forked repo.

Please consider this merge request as "request for comments" on my changes:

  • only one "owning" string member, the other members are non-owning and just point to certain parts of the (processed) URL.
  • rename API functions to use the official names for the parts of a URL:
    • protocol -> scheme
    • hash -> fragment
  • remove the functions / members with unclear semantics
    • path vs. pathname
    • host vs. hostname
    • query vs. search
  • document which members might be NULL
  • add support for IPv6 literals in hostname

Features still missing but shall come in future:

  • decode percend-encoded octets (%xx)
  • parse query string into list of name-value pairs
  • (maybe) IDN support
  • correct path parser, incl. path normalization ("/foo/../bar/./yuk" -> "/bar/yuk")

roker and others added 22 commits June 13, 2023 18:13
merge bumped version from upstream
@RokerHRO RokerHRO marked this pull request as draft December 8, 2023 08:49
@jwerle
Copy link
Owner

jwerle commented Dec 15, 2023

@RokerHRO very nice work! let me know when you are ready for review!

@RokerHRO
Copy link
Collaborator Author

RokerHRO commented Dec 15, 2023

@RokerHRO very nice work! let me know when you are ready for review!

Thank you!

You can already review and comment the changes I made! :-)

What I'd still like to add is parsing the query string (incl. %-decoding) into an array (or linked list? what would you prefer?) of key-value pairs and adding access function for that.

@RokerHRO RokerHRO marked this pull request as ready for review December 27, 2023 15:07
@RokerHRO
Copy link
Collaborator Author

RokerHRO commented Dec 27, 2023

@jwerle : I'd say it is ready for review now. :-)

@RokerHRO
Copy link
Collaborator Author

RokerHRO commented Jan 7, 2024

The version number should be bumped, because my rewrite results in incompatible API change. ;-)

@RokerHRO
Copy link
Collaborator Author

RokerHRO commented Jun 9, 2024

@jwerle : are you still active and interested in this project?

@jwerle jwerle merged commit 752635e into jwerle:master Jun 14, 2024
@jwerle
Copy link
Owner

jwerle commented Jun 14, 2024

@jwerle : are you still active and interested in this project?

I am interested, but not active on it. Just merged. Feel free to take lead on this!

@RokerHRO RokerHRO deleted the change_member_ownership branch June 14, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants