Improve string formatting error messages

- 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.
This commit is contained in:
Hugo Locurcio 2024-08-13 21:07:16 +02:00
parent 06fbc8395b
commit 3b012f1fac
No known key found for this signature in database
GPG Key ID: 39E8F8BE30B0A49C
2 changed files with 28 additions and 28 deletions

View File

@ -5352,11 +5352,11 @@ String String::sprintf(const Array &values, bool *error) const {
case 'x': // Hexadecimal (lowercase) case 'x': // Hexadecimal (lowercase)
case 'X': { // Hexadecimal (uppercase) case 'X': { // Hexadecimal (uppercase)
if (value_index >= values.size()) { 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()) { 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]; int64_t value = values[value_index];
@ -5418,11 +5418,11 @@ String String::sprintf(const Array &values, bool *error) const {
} }
case 'f': { // Float case 'f': { // Float
if (value_index >= values.size()) { 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()) { 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]; double value = values[value_index];
@ -5463,7 +5463,7 @@ String String::sprintf(const Array &values, bool *error) const {
} }
case 'v': { // Vector2/3/4/2i/3i/4i case 'v': { // Vector2/3/4/2i/3i/4i
if (value_index >= values.size()) { 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; int count;
@ -5481,7 +5481,7 @@ String String::sprintf(const Array &values, bool *error) const {
count = 4; count = 4;
} break; } break;
default: { 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()));
} }
} }
@ -5533,7 +5533,7 @@ String String::sprintf(const Array &values, bool *error) const {
} }
case 's': { // String case 's': { // String
if (value_index >= values.size()) { 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]; String str = values[value_index];
@ -5551,7 +5551,7 @@ String String::sprintf(const Array &values, bool *error) const {
} }
case 'c': { case 'c': {
if (value_index >= values.size()) { 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. // Convert to character.
@ -5559,20 +5559,20 @@ String String::sprintf(const Array &values, bool *error) const {
if (values[value_index].is_num()) { if (values[value_index].is_num()) {
int value = values[value_index]; int value = values[value_index];
if (value < 0) { 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) { } 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) { } 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]); str = chr(values[value_index]);
} else if (values[value_index].get_type() == Variant::STRING) { } else if (values[value_index].get_type() == Variant::STRING) {
str = values[value_index]; str = values[value_index];
if (str.length() != 1) { 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 { } 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. // Padding.
@ -5616,7 +5616,7 @@ String String::sprintf(const Array &values, bool *error) const {
} else { } else {
if (c == '0' && min_chars == 0) { if (c == '0' && min_chars == 0) {
if (left_justified) { 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 { } else {
pad_with_zeros = true; pad_with_zeros = true;
} }
@ -5629,7 +5629,7 @@ String String::sprintf(const Array &values, bool *error) const {
} }
case '.': { // Float/Vector separator. case '.': { // Float/Vector separator.
if (in_decimals) { 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; in_decimals = true;
min_decimals = 0; // We want to add the value manually. min_decimals = 0; // We want to add the value manually.
@ -5638,7 +5638,7 @@ String String::sprintf(const Array &values, bool *error) const {
case '*': { // Dynamic width, based on value. case '*': { // Dynamic width, based on value.
if (value_index >= values.size()) { 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(); Variant::Type value_type = values[value_index].get_type();
@ -5646,7 +5646,7 @@ String String::sprintf(const Array &values, bool *error) const {
value_type != Variant::VECTOR2 && value_type != Variant::VECTOR2I && value_type != Variant::VECTOR2 && value_type != Variant::VECTOR2I &&
value_type != Variant::VECTOR3 && value_type != Variant::VECTOR3I && value_type != Variant::VECTOR3 && value_type != Variant::VECTOR3I &&
value_type != Variant::VECTOR4 && value_type != Variant::VECTOR4I) { 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]; int size = values[value_index];
@ -5662,7 +5662,7 @@ String String::sprintf(const Array &values, bool *error) const {
} }
default: { default: {
return "unsupported format character"; return vformat("Unsupported format character \"%%%c\".", c);
} }
} }
} else { // Not in format string. } else { // Not in format string.
@ -5684,11 +5684,11 @@ String String::sprintf(const Array &values, bool *error) const {
} }
if (in_format) { 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()) { 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) { if (error) {

View File

@ -1111,7 +1111,7 @@ TEST_CASE("[String] sprintf") {
args.push_back("cheese"); args.push_back("cheese");
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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. // More arguments than formats.
format = "fish %s frog"; format = "fish %s frog";
@ -1120,7 +1120,7 @@ TEST_CASE("[String] sprintf") {
args.push_back("cheese"); args.push_back("cheese");
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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. // Incomplete format.
format = "fish %10"; format = "fish %10";
@ -1128,7 +1128,7 @@ TEST_CASE("[String] sprintf") {
args.push_back("cheese"); args.push_back("cheese");
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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. // Bad character in format string.
format = "fish %&f frog"; format = "fish %&f frog";
@ -1136,7 +1136,7 @@ TEST_CASE("[String] sprintf") {
args.push_back("cheese"); args.push_back("cheese");
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(error); REQUIRE(error);
CHECK(output == "unsupported format character"); CHECK(output == "Unsupported format character \"%&\".");
// Too many decimals. // Too many decimals.
format = "fish %2.2.2f frog"; format = "fish %2.2.2f frog";
@ -1144,7 +1144,7 @@ TEST_CASE("[String] sprintf") {
args.push_back(99.99); args.push_back(99.99);
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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. // * not a number or vector.
format = "fish %*f frog"; format = "fish %*f frog";
@ -1153,7 +1153,7 @@ TEST_CASE("[String] sprintf") {
args.push_back(99.99); args.push_back(99.99);
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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. // Character too long.
format = "fish %c frog"; format = "fish %c frog";
@ -1161,7 +1161,7 @@ TEST_CASE("[String] sprintf") {
args.push_back("sc"); args.push_back("sc");
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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. // Character bad type.
format = "fish %c frog"; format = "fish %c frog";
@ -1169,7 +1169,7 @@ TEST_CASE("[String] sprintf") {
args.push_back(Array()); args.push_back(Array());
output = format.sprintf(args, &error); output = format.sprintf(args, &error);
REQUIRE(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") { TEST_CASE("[String] is_numeric") {