Closed markeel closed 7 months ago
Great, what does it take to release it into the asset library?
@markeel Can you make the requested changes? I'll merge this afterwards.
Yes, I'll take a look at it tomorrow morning.
On Wed, Apr 3, 2024, 8:36 AM Hugo Locurcio @.***> wrote:
@markeel https://github.com/markeel Can you make the requested changes? I'll merge this afterwards.
— Reply to this email directly, view it on GitHub https://github.com/godotengine/godot-git-plugin/pull/229#issuecomment-2034945337, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJWU3MV2D7X2FWXYKI6K6LDY3QOXHAVCNFSM6AAAAABFSFTIM2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZUHE2DKMZTG4 . You are receiving this because you were mentioned.Message ID: @.***>
I incorporated the items @bruvzg identified in his review. Those were subtle impacts. The only change I didn't apply was the one in git_callbacks related to username. The trickiness with this fix, is whether the string was already a "godot" string. In the case of the username it was coming from credentials and therefore was a "godot" string. When it comes from the URL the string cannot be UTF-8 since those are invalid characters in a URL.
During testing (where I created repositories and branch names with emojis in the name) I ran into problems where the messages being logged to the "Output" window using the UtilityFunction::print(...) methods needed to be updated. Many of the calls were converting from a "godot" string back to a CString, but then the print function would convert it back to a "godot" string but not convert the C string as a UTF-8. Since the print() method takes godot string it was more accurate and effective to remove this extraneous conversion to CString.
I also tested error legs, by shutting down the server running my remote repositories to see how pull and push reported errors.
Re-implemented the original update for UTF-8 based on latest master, for a clean merge.
Replaced the places where the libgit2 API used godot::String with godot::String::utf8
Tested using emojis as place holders for language specific Unicode characters