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

Docs: Link to GlobalScope string methods from String class ref #98473

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

tetrapod00
Copy link
Contributor

Closes godotengine/godot-docs#10132. If this PR is not approved, the original issue should be closed as not planned.

Adds links to the @GlobalScope functions str(), str_to_var(), and var_to_str() from the String class ref. These are the only three @GlobalScope functions that are clearly related to strings, rather than just using strings incidentally.

These three functions are of interest to users looking at the string documentation. These functions could just as easily have been implemented as static functions String.from(), String.to_var(), and String.from_var(). So it's due to implementation choices and the class reference's inflexibility that these functions are not currently shown on the String class reference. All that, and the fact that at least one user noticed their absence in the String class ref, makes them worth linking.

I personally don't mind the status quo in which these functions are not linked from the String page. One way or another, this PR should close the original issue, as either implemented or not planned.

@tetrapod00 tetrapod00 requested a review from a team as a code owner October 23, 2024 21:26
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change seems fine to me

@clayjohn clayjohn added enhancement documentation cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Oct 23, 2024
@clayjohn clayjohn added this to the 4.4 milestone Oct 23, 2024
@BrianBHuynh
Copy link
Contributor

Huh we were actually just talking about this problem in the discord recently

Copy link
Contributor

@BrianBHuynh BrianBHuynh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:D looks good

@@ -6,6 +6,7 @@
<description>
This is the built-in string Variant type (and the one used by GDScript). Strings may contain any number of Unicode characters, and expose methods useful for manipulating and generating strings. Strings are reference-counted and use a copy-on-write approach (every modification to a string returns a new [String]), so passing them around is cheap in resources.
Some string methods have corresponding variations. Variations suffixed with [code]n[/code] ([method countn], [method findn], [method replacen], etc.) are [b]case-insensitive[/b] (they make no distinction between uppercase and lowercase letters). Method variations prefixed with [code]r[/code] ([method rfind], [method rsplit], etc.) are reversed, and start from the end of the string, instead of the beginning.
To convert any Variant to or from a string, see [method @GlobalScope.str], [method @GlobalScope.str_to_var], and [method @GlobalScope.var_to_str].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To convert any Variant to or from a string, see [method @GlobalScope.str], [method @GlobalScope.str_to_var], and [method @GlobalScope.var_to_str].
To convert any [Variant] to or from a string, see [method @GlobalScope.str], [method @GlobalScope.str_to_var], and [method @GlobalScope.var_to_str].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oop, missed this before merging, my b

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I make another PR to fix or does this go in #91521 now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added it to the tracker since it seems to have been committed like this for a few weeks now. Ideally minor cosmetic mistakes like these shouldn't result in making a new PR fixing just that, but instead done in batches ^^

@Repiteo Repiteo merged commit 28f56eb into godotengine:master Oct 24, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 24, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation for str() and other secret functions
6 participants