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

Standardize all "Prints" comments in documentation #99876

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Nov 30, 2024

Related to

This PR tries to standardize the common # Prints comment present in a lot of the class reference. This is based off of a few rules, which are taken from my own written attempt to "explain myself":

  • Where appropriate, use the following common inline comments:
    • # Prints {value}: After a print call. (or similar).
      Suitable for beginner topics, where you want the user to see the result upon copying and pasting the code sample. {value} should be formatted exactly how it would be printed to the console. No trailing period.
      • Do not encompass in quotes, unless a single String/StringName is the argument for print.
      • Encompass everything in quotes, when multiple arguments in the same print are used.
      • Beware of C# differences. GD.Print() outputs "True" and "False", trims the trailing .0, and outputs enum constants' names instead of their value.

This PR is still a draft because these rules are still not consistent.


As of writing this, not many additional changes are done to the affected descriptions.
Note that, in the grand scheme of things, this is an extremely minor change. Approve and merge this PR only if you feel like this is worth the localization needing to be refreshed.

@Mickeon Mickeon added this to the 4.x milestone Nov 30, 2024
@Mickeon Mickeon force-pushed the documentation-prints-obsession branch from 1a67266 to 6b35fd1 Compare November 30, 2024 14:57
@Repiteo
Copy link
Contributor

Repiteo commented Dec 3, 2024

Given the mentioned PRs fall under a similar umbrella, and this is already a niche change, I'd recommend consolidating all of them into this PR

@Mickeon
Copy link
Contributor Author

Mickeon commented Dec 3, 2024

Out of these kinds of PRs I decided to keep #99081 separate because the future of #47502 is still a bit murky. Not sure it should be merged into once batch but I can do it just fine.

@Mickeon Mickeon force-pushed the documentation-prints-obsession branch from 6b35fd1 to dab4a5c Compare December 14, 2024 17:43
@Mickeon Mickeon marked this pull request as ready for review December 14, 2024 17:43
@Mickeon Mickeon requested a review from a team as a code owner December 14, 2024 17:43
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

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

The changes to prints do seem to match the standards laid out in the OP. The other improvements made along the way generally look okay to me.

doc/classes/Array.xml Outdated Show resolved Hide resolved
doc/classes/ResourceLoader.xml Show resolved Hide resolved
@Mickeon Mickeon force-pushed the documentation-prints-obsession branch from dab4a5c to 1fb50ad Compare December 14, 2024 21:30
@@ -849,11 +849,11 @@
[codeblocks]
[gdscript]
var a = [1, 2, 3]
print("a", "b", a) # Prints ab[1, 2, 3]
print("a", "b", a) # Prints "ab[1, 2, 3]"
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this example specifically, I wonder if a policy that is to use quotes for strings but not for number types is the right one.

What gets printed is:

ab[1, 2, 3]

which doesn't differ much from the output of print(42) or print("ab[1, 2, 3]").

var a = [1, 2, 3]
print("a", "b", a)
print("ab[1, 2, 3]")
print(42)

prints:

ab[1, 2, 3]
ab[1, 2, 3]
42

So maybe we should use quotes all the time and not just for strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave that up for you or someone else to decide. Doing this is bikeshedding a bit and would be a whole lot more lines to change, but I do not mind doing it so long as it's consistent.

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.

4 participants