-
-
Notifications
You must be signed in to change notification settings - Fork 373
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
🚀 Add number format expression #4663
🚀 Add number format expression #4663
Conversation
Co-authored-by: LimeGlass <[email protected]>
Marking as Draft until I find a fix for the syntax conflict with ExprFormatDate (when using variables) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering whether this might actually be better as a built-in function (of the style formatNumber(number, pattern)
), rather than as a syntax.
My concerns are threefold:
- This a
%thing% ... %thing
syntax pattern, which is painful to parse. - The syntax feels a little clumsy, e.g.
10 formatted human-readable with "###"
doesn't sound very English. This isn't your fault; I'm not sure if there is a nice way to phrase this. - I'm worried people might confuse 'formatted as' with 'parsed as' and try to misuse this, especially since it's specific to numbers currently.
This is just a suggestion, if you feel syntax is better than a function then that is fine.
This is a very good suggestion.. it will fix the major parsing issue of this syntax and formatted date syntax because they both are identical. |
bd134d0
to
3f08853
Compare
src/main/java/ch/njol/skript/classes/data/DefaultFunctions.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/DefaultFunctions.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
"command /formatnumber <number>:", | ||
"\taliases: fn", | ||
"\ttrigger:", | ||
"\t\tsend \"Formatted: %formatNumber(arg-1)%\" to sender" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more examples of how to use DecimalFormat would be nice. The table on the javadocs is good, but pretty dense for new users.
@@ -0,0 +1,17 @@ | |||
test "formatted numbers function": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure to test invalid numbers too! NaN, infinity, null values, etc.
Floating point would also be good to show.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good apart from sovde's requested changes. feel free to re-request my review after those changes are made.
Continued in #7166 |
Description
Add a new function to format numbers like
123,456,789
Target Minecraft Versions: Any
Requirements: None
Related Issues: None