Skip to content

Commit

Permalink
Improve string formatting error messages
Browse files Browse the repository at this point in the history 
- Print additional context to make troubleshooting easier.
- Add the placeholder where the error occurred as a prefix to the error message.
- Use sentence case for all error messages.
  • Loading branch information
@Calinou
Calinou committed Nov 20, 2024
1 parent 9e60984 commit cbe895c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 28 deletions.
40 changes: 20 additions & 20 deletions core/string/ustring.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5618,11 +5618,11 @@ String String::sprintf(const Array &values, bool *error) const {
case 'x': // Hexadecimal (lowercase)
case 'X': { // Hexadecimal (uppercase)
if (value_index >= values.size()) {
return "not enough arguments for format string";
return vformat("[%%%c] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", c, value_index);
}

if (!values[value_index].is_num()) {
return "a number is required";
return vformat("[%%%c] A number (int/float) is required, but a %s was provided.", c, Variant().get_type_name(values[value_index].get_type()));
}

int64_t value = values[value_index];
Expand Down Expand Up @@ -5684,11 +5684,11 @@ String String::sprintf(const Array &values, bool *error) const {
}
case 'f': { // Float
if (value_index >= values.size()) {
return "not enough arguments for format string";
return vformat("[%%f] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index);
}

if (!values[value_index].is_num()) {
return "a number is required";
return vformat("[%%f] A number (int/float) is required, but a %s was provided.", Variant().get_type_name(values[value_index].get_type()));
}

double value = values[value_index];
Expand Down Expand Up @@ -5729,7 +5729,7 @@ String String::sprintf(const Array &values, bool *error) const {
}
case 'v': { // Vector2/3/4/2i/3i/4i
if (value_index >= values.size()) {
return "not enough arguments for format string";
return vformat("[%%v] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index);
}

int count;
Expand All @@ -5747,7 +5747,7 @@ String String::sprintf(const Array &values, bool *error) const {
count = 4;
} break;
default: {
return "%v requires a vector type (Vector2/3/4/2i/3i/4i)";
return vformat("[%%v]: A vector is required (Vector2/3/4/2i/3i/4i), but a %s was provided.", Variant().get_type_name(values[value_index].get_type()));
}
}

Expand Down Expand Up @@ -5799,7 +5799,7 @@ String String::sprintf(const Array &values, bool *error) const {
}
case 's': { // String
if (value_index >= values.size()) {
return "not enough arguments for format string";
return vformat("[%%s] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index);
}

String str = values[value_index];
Expand All @@ -5817,28 +5817,28 @@ String String::sprintf(const Array &values, bool *error) const {
}
case 'c': {
if (value_index >= values.size()) {
return "not enough arguments for format string";
return vformat("[%%c] Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index);
}

// Convert to character.
String str;
if (values[value_index].is_num()) {
int value = values[value_index];
if (value < 0) {
return "unsigned integer is lower than minimum";
return vformat("[%%c]: Unsigned integer (-0x%x) is lower than the minimum allowed value (0).", Math::abs(value));
} else if (value >= 0xd800 && value <= 0xdfff) {
return "unsigned integer is invalid Unicode character";
return vformat("[%%c]: Unsigned integer (0x%x) is an invalid Unicode character.", value);
} else if (value > 0x10ffff) {
return "unsigned integer is greater than maximum";
return vformat("[%%c]: Unsigned integer (0x%x) is greater than the maximum allowed value (0x10ffff).", value);
}
str = chr(values[value_index]);
} else if (values[value_index].get_type() == Variant::STRING) {
str = values[value_index];
if (str.length() != 1) {
return "%c requires number or single-character string";
return vformat("[%%c]: A number or a single-character string is required, but a string of %d characters was provided.", str.length());
}
} else {
return "%c requires number or single-character string";
return vformat("[%%c]: A number or a single-character string is required, but a %s was provided.", Variant().get_type_name(values[value_index].get_type()));
}

// Padding.
Expand Down Expand Up @@ -5882,7 +5882,7 @@ String String::sprintf(const Array &values, bool *error) const {
} else {
if (c == '0' && min_chars == 0) {
if (left_justified) {
WARN_PRINT("'0' flag ignored with '-' flag in string format");
WARN_PRINT("[%0]: The \"0\" flag is ignored with \"-\" flag in string formatting.");
} else {
pad_with_zeros = true;
}
Expand All @@ -5895,7 +5895,7 @@ String String::sprintf(const Array &values, bool *error) const {
}
case '.': { // Float/Vector separator.
if (in_decimals) {
return "too many decimal points in format";
return "[%.]: Too many decimal points in format (only 1 point is allowed).";
}
in_decimals = true;
min_decimals = 0; // We want to add the value manually.
Expand All @@ -5904,15 +5904,15 @@ String String::sprintf(const Array &values, bool *error) const {

case '*': { // Dynamic width, based on value.
if (value_index >= values.size()) {
return "not enough arguments for format string";
return vformat("[%*]: Not enough arguments for string formatting (%d provided, but more were expected). Check for extraneous %% placeholders or missing arguments.", value_index);
}

Variant::Type value_type = values[value_index].get_type();
if (!values[value_index].is_num() &&
value_type != Variant::VECTOR2 && value_type != Variant::VECTOR2I &&
value_type != Variant::VECTOR3 && value_type != Variant::VECTOR3I &&
value_type != Variant::VECTOR4 && value_type != Variant::VECTOR4I) {
return "* wants number or vector";
return vformat("[%*]: A number (int/float) or a vector type (Vector2/3/4/2i/3i/4i) is required, but a %s was provided.", Variant().get_type_name(values[value_index].get_type()));
}

int size = values[value_index];
Expand All @@ -5928,7 +5928,7 @@ String String::sprintf(const Array &values, bool *error) const {
}

default: {
return "unsupported format character";
return vformat("Unsupported format character \"%%%c\".", c);
}
}
} else { // Not in format string.
Expand All @@ -5950,11 +5950,11 @@ String String::sprintf(const Array &values, bool *error) const {
}

if (in_format) {
return "incomplete format";
return "Incomplete format string. Check that all % placeholders use the correct syntax and are terminated correctly.";
}

if (value_index != values.size()) {
return "not all arguments converted during string formatting";
return vformat("Not all arguments were converted during string formatting (%d provided, but %d were expected). Check for missing %% placeholders or extraneous arguments.", values.size(), value_index);
}

if (error) {
Expand Down
16 changes: 8 additions & 8 deletions tests/core/string/test_string.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@ TEST_CASE("[String] sprintf") {
args.push_back("cheese");
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "not enough arguments for format string");
CHECK(output == "[%s] Not enough arguments for string formatting (1 provided, but more were expected). Check for extraneous % placeholders or missing arguments.");

// More arguments than formats.
format = "fish %s frog";
Expand All @@ -1188,31 +1188,31 @@ TEST_CASE("[String] sprintf") {
args.push_back("cheese");
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "not all arguments converted during string formatting");
CHECK(output == "Not all arguments were converted during string formatting (2 provided, but 1 were expected). Check for missing % placeholders or extraneous arguments.");

// Incomplete format.
format = "fish %10";
args.clear();
args.push_back("cheese");
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "incomplete format");
CHECK(output == "Incomplete format string. Check that all % placeholders use the correct syntax and are terminated correctly.");

// Bad character in format string.
format = "fish %&f frog";
args.clear();
args.push_back("cheese");
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "unsupported format character");
CHECK(output == "Unsupported format character \"%&\".");

// Too many decimals.
format = "fish %2.2.2f frog";
args.clear();
args.push_back(99.99);
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "too many decimal points in format");
CHECK(output == "[%.]: Too many decimal points in format (only 1 point is allowed).");

// * not a number or vector.
format = "fish %*f frog";
Expand All @@ -1221,23 +1221,23 @@ TEST_CASE("[String] sprintf") {
args.push_back(99.99);
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "* wants number or vector");
CHECK(output == "[%*]: A number (int/float) or a vector type (Vector2/3/4/2i/3i/4i) is required, but a String was provided.");

// Character too long.
format = "fish %c frog";
args.clear();
args.push_back("sc");
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "%c requires number or single-character string");
CHECK(output == "[%c]: Requires a number or a single-character string, but a string of 2 characters was provided.");

// Character bad type.
format = "fish %c frog";
args.clear();
args.push_back(Array());
output = format.sprintf(args, &error);
REQUIRE(error);
CHECK(output == "%c requires number or single-character string");
CHECK(output == "[%c]: A number or a single-character string is required, but a Array was provided.");
}

TEST_CASE("[String] is_numeric") {
Expand Down

0 comments on commit cbe895c

Please sign in to comment.