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

Expose GodotString formatting facility #816

Merged
merged 1 commit into from Nov 27, 2021

Conversation

mivort
Copy link
Contributor

@mivort mivort commented Nov 11, 2021

This PR exposes godot_string_format API method to make built-in Godot string formatting available in GDNative API. It also adds a basic formatting in-engine test case.

@Bromeon
Copy link
Member

Bromeon commented Nov 11, 2021

Thanks! 🙂

With the suggested API, we don't support the placeholder argument, but I think that one is used so rarely that it should be OK. We can always add an "overload" later.

Could you maybe add an assert_eq! in the test that makes sure that the old string is unchanged? Just to be sure, and also for expressing the expected behavior.

Also, since the behavior is a bit special (see here) and format() accepts both arrays and dictionaries, do you think it makes sense to document the method with 2 examples, one for array and one for dictionary? Maybe also add a quick array test.

@Bromeon Bromeon added feature Adds functionality to the library c: core Component: core (mod core_types, object, log, init, ...) labels Nov 11, 2021
@Bromeon Bromeon added this to the v0.10 milestone Nov 11, 2021
@mivort
Copy link
Contributor Author

mivort commented Nov 11, 2021

Do you think it makes sense to document the method with 2 examples, one for array and one for dictionary? Maybe also add a quick array test.

Sure, documentation always may come in handy :) I've added examples for index-style array values and dictionary, and also added test assertions for original string and array.

@mivort mivort force-pushed the godot_string_format branch 2 times, most recently from b9cb1b7 to 63b5b08 Compare November 11, 2021 23:03
This PR exposes `godot_string_format` API method to make built-in Godot
string formatting available in GDNative API. It also adds a basic
formatting in-engine test case.

Signed-off-by: Jan Haller <bromeon@gmail.com>
@Bromeon
Copy link
Member

Bromeon commented Nov 27, 2021

Combined the commits and changed ```ignore to ```no_run, so we can run compile-time doc tests on the examples. Retained author information with signed-off-by.

Thanks again!
bors r+

@bors
Copy link
Contributor

bors bot commented Nov 27, 2021

Build succeeded:

@bors bors bot merged commit 164beb8 into godot-rust:master Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Component: core (mod core_types, object, log, init, ...) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants