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

Unify URI encoding/decoding, handle spaces-are-pluses, and handle hex/bin prefix automatically #43978

Merged
merged 2 commits into from
Jan 28, 2021

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Nov 30, 2020

Made in response to #43929. The first two bullet points are one commit, the last three are the second commit.

  • Make String::hex_to_int() and String::bin_to_int() automatically handle the prefix. This simplifies the code, both internally and for the end user. The p_with_prefix argument was not exposed in 3.2, so this doesn't break compat.

  • Add BinToInt instance method to string in C#.

  • Unify http_escape and percent_encode into uri_encode, and http_unescape and percent_decode into uri_decode.
    This restores part of Enhance uri utils #17583 and un-reverts that part's reversion Revert "Unify http- and percent- encode/decode" #18156 (see either of these for discussion).

  • Tweak String::uri_decode() so that it handles spaces encoded as +.

  • Add URIEncode and URIDecode instance methods to string in C#.

Me making this PR:

@aaronfranke aaronfranke added this to the 4.0 milestone Nov 30, 2020
@aaronfranke aaronfranke requested a review from a team as a code owner November 30, 2020 06:29
@aaronfranke aaronfranke force-pushed the cs-string branch 2 times, most recently from 7c69ecf to 070d7dd Compare November 30, 2020 09:59
@aaronfranke aaronfranke changed the title Add HTTP(Un)Escape to C#, handle spaces-are-pluses, and handle hex/bin prefix automatically Unify URI encoding/decoding, handle spaces-are-pluses, and handle hex/bin prefix automatically Nov 30, 2020
@aaronfranke aaronfranke force-pushed the cs-string branch 2 times, most recently from 70158d5 to 529c0e5 Compare November 30, 2020 10:31
@aaronfranke aaronfranke requested review from RandomShaper and removed request for hpvb and vnen November 30, 2020 11:02
@akien-mga
Copy link
Member

  • Make String::hex_to_int() and String::bin_to_int() automatically handle the prefix. This simplifies the code, both internally and for the end user. The p_with_prefix argument was not exposed in 3.2, so this doesn't break compat.

From the code and docs, it seems like this does break compat as the argument was exposed. But I'm still OK with the change for 4.0.

@akien-mga
Copy link
Member

  • Make String::hex_to_int() and String::bin_to_int() automatically handle the prefix. This simplifies the code, both internally and for the end user. The p_with_prefix argument was not exposed in 3.2, so this doesn't break compat.

From the code and docs, it seems like this does break compat as the argument was exposed. But I'm still OK with the change for 4.0.

The original statement was correct, it's not exposed in 3.2 indeed. It was exposed in 4.0 with the Variant refactor, but it's fine to unexpose it.

http_escape and percent_encode have been unified into uri_encode, and http_unescape and percent_decode have been unified into uri_decode.
@akien-mga akien-mga merged commit e50422d into godotengine:master Jan 28, 2021
@akien-mga
Copy link
Member

Thanks!

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