Skip to content

More formatting options #21

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

More formatting options #21

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fa88be
Add `format` and `formatDateTime`
Xenapte Apr 23, 2024
3cbaafd
Merge branch 'MeteorDevelopment:master' into master
Xenapte Jun 28, 2024
df7b249
fix null formatting
Xenapte Jun 28, 2024
f4251b6
add option to specify timezone
Xenapte Jun 28, 2024
07567ba
style improvements
Xenapte Jun 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/main/java/org/meteordev/starscript/StandardLib.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import org.meteordev.starscript.value.Value;

import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;
import java.util.IllegalFormatException;
import java.util.Random;

/** Standard library with some default functions and variables. */
Expand Down Expand Up @@ -35,6 +37,10 @@ public static void init(Starscript ss) {
ss.set("contains", StandardLib::contains);
ss.set("replace", StandardLib::replace);
ss.set("pad", StandardLib::pad);

// Formatters
ss.set("formatDateTime", StandardLib::formatDateTime);
ss.set("format", StandardLib::format);
}

// Numbers
Expand Down Expand Up @@ -166,4 +172,44 @@ public static Value pad(Starscript ss, int argCount) {

return Value.string(new String(padded));
}

public static Value formatDateTime(Starscript ss, int argCount) {
if (argCount != 1) ss.error("formatTime() requires 1 argument, got %d.", argCount);
try {
String fmt = ss.popString("Argument to formatTime(fmt) needs to be a string.");
SimpleDateFormat formatter = new SimpleDateFormat(fmt);
return Value.string(formatter.format(new Date()));
}
catch (IllegalArgumentException e) {
ss.error(e.toString());
}
return Value.null_();
}

public static Value format(Starscript ss, int argCount) {
if (argCount < 1) ss.error("format(fmt, ...args) requires at least 1 argument, got %d.", argCount);
ArrayList<Object> args = new ArrayList<Object>();
Copy link
Contributor

Choose a reason for hiding this comment

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

the current implementation is less than ideal in performance

  1. using the no-arg ArrayList constructor means this method will potentially cause several object arrays to be allocated as the list will have to be resized
  2. inserting objects at index 0 means that the other elements will have to be offset constantly

it would be better to just use an object array directly and filling it with a reverse for i loop

for (int i = 1; i < argCount; i ++) {
Value v = ss.pop();
Object o = null;
switch (v.type) {
case Null: o = Value.null_(); break;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is wrong, should just be o = null

Copy link
Author

Choose a reason for hiding this comment

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

this is wrong, should just be o = null

iirc I used this because I saw Value.null_() being used somewhere else. Anyways do you want me to edit and fix this, if you are going to accept this pr?

Copy link

Choose a reason for hiding this comment

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

For me personally it would be really nice, if this PR is being merged. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

this is wrong, should just be o = null

iirc I used this because I saw Value.null_() being used somewhere else. Anyways do you want me to edit and fix this, if you are going to accept this pr?

Value.null_() is the starscript null type, but in this case you’re feeding it into the java String.format() method, which takes in the java null

Copy link
Author

Choose a reason for hiding this comment

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

Check my new commits in case you're interested in accepting this PR. Should be fixed now.

case Boolean: o = v.getBool(); break;
case Number: o = v.getNumber(); break;
case String: o = v.getString(); break;
case Function: o = v.getFunction(); break;
case Map: o = v.getMap(); break;
default: break;
}
args.add(0, o);
}
String fmt = ss.popString("Argument `fmt` to format() needs to be a string.");
try {
return Value.string(String.format(fmt, args.toArray()));
}
catch (IllegalFormatException e) {
ss.error(e.toString());
}
return Value.null_();
}
}