From b3569d9d86d63e3525d68ace5b8faf12c9b23e03 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 29 Oct 2026 10:52:47 -0400 Subject: [PATCH 1/5] Display only windows of a line in error formatting When an error is very far into a line, we were previously displaying the whole context line above it and then padding out spaces before the error message. For pathological cases, this ends up being really bad. Instead we now display errors under a window of context. --- src/errors_format.c | 71 +++++++++++++++++++----------- test/prism/api/raise_error_test.rb | 17 +++++++ 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/src/errors_format.c b/src/errors_format.c index cdde17162b..0785d90d46 100644 --- a/src/errors_format.c +++ b/src/errors_format.c @@ -73,6 +73,11 @@ pm_rich_error_compare(const void *a, const void *b) { #define COLOR_RESET "\033[m" #define TRUNCATE 30 +/* The maximum number of columns of a line that errors are displayed beneath + * before the line is displayed again. This must be at least TRUNCATE so that + * every window after the first on a line starts with a truncated line. */ +#define WINDOW 80 + typedef struct { const char *number_prefix; const char *blank_prefix; @@ -189,15 +194,21 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const char pm_buffer_append_format(buffer, number_prefix, line); + /* Here we determine if we should truncate the start of the line. The + * column is the start of a diagnostic, so it is on a character boundary. */ + bool truncate_start = column_start >= TRUNCATE; + const uint8_t *display_start = truncate_start ? (start + column_start) : start; + /* Here we determine if we should truncate the end of the line. Note that * this is written to avoid computing start + column_end, which could be * more than one past the end of the source when the error is at the end - * of the input. */ + * of the input. The walk to find a character boundary begins at the + * displayed start so that its cost is bounded by the displayed width. */ bool truncate_end = false; if ((column_end != 0) && ((end - start) - ((ptrdiff_t) column_end) >= TRUNCATE)) { const uint8_t *end_candidate = start + column_end + TRUNCATE; - for (const uint8_t *cursor = start; cursor < end_candidate;) { + for (const uint8_t *cursor = display_start; cursor < end_candidate;) { size_t char_width = pm_parser_encoding_char_width(parser, cursor, parser->end - cursor); /* If we failed to decode a character, then just bail out and @@ -218,13 +229,8 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const char truncate_end = true; } - /* Here we determine if we should truncate the start of the line. */ - if (column_start >= TRUNCATE) { - pm_buffer_append_string(buffer, "... ", 4); - start += column_start; - } - - pm_buffer_append_string(buffer, (const char *) start, (size_t) (end - start)); + if (truncate_start) pm_buffer_append_string(buffer, "... ", 4); + pm_buffer_append_string(buffer, (const char *) display_start, (size_t) (end - display_start)); if (truncate_end) { pm_buffer_append_string(buffer, " ...\n", 5); @@ -242,9 +248,17 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ * it. While we are iterating, we will display some padding lines of the * source before the error to give some context. We will be careful not to * display the same line twice in case the errors are close enough in the - * source. */ + * source. + * + * The errors on a line are split into windows of at most WINDOW columns. + * Each window displays its own slice of the line, and the markers beneath + * it are padded relative to the start of that slice and cut off at its + * end. This bounds the length of every marker line, so the output grows + * linearly with the number of errors no matter how long the line is. */ int32_t last_line = parser->start_line - 1; - uint32_t last_column_start = 0; + uint32_t window_column_start = 0; + uint32_t window_column_end = 0; + size_t window_end_idx = 0; for (size_t idx = 0; idx < nerrors; idx++) { pm_rich_error_t *rich_error = rich_errors + idx; @@ -265,9 +279,10 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line - 1, 0, 0); } - /* If this is the first error or we are on a new line, then we will - * display the line that has the error in it. */ - if ((idx == 0) || (rich_error->line != last_line)) { + /* If this error is not in the current window (because it is the first + * error, it is on a new line, or it is too far to the right), then we + * start a new window and display the line that has the error in it. */ + if (idx == window_end_idx) { switch (format_type) { case PM_ERRORS_FORMAT_PLAIN: pm_buffer_append_string(buffer, "> ", 2); @@ -280,16 +295,22 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ break; } - last_column_start = rich_error->column_start; - - /* Find the maximum column end of all the errors on this line. */ - uint32_t column_end = rich_error->column_end; - for (size_t next_idx = idx + 1; next_idx < nerrors; next_idx++) { - if (rich_errors[next_idx].line != rich_error->line) break; - if (rich_errors[next_idx].column_end > column_end) column_end = rich_errors[next_idx].column_end; + /* The displayed slice of the line starts at this error, unless it + * is close enough to the start of the line to show it all. */ + window_column_start = (rich_error->column_start >= TRUNCATE) ? rich_error->column_start : 0; + uint32_t window_limit = window_column_start + WINDOW; + + /* Find the errors on this line that start within the window, and + * the maximum column end of all of them, capped at the window. */ + window_column_end = rich_error->column_end; + for (window_end_idx = idx + 1; window_end_idx < nerrors; window_end_idx++) { + const pm_rich_error_t *next_error = &rich_errors[window_end_idx]; + if ((next_error->line != rich_error->line) || (next_error->column_start >= window_limit)) break; + if (next_error->column_end > window_column_end) window_column_end = next_error->column_end; } - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line, rich_error->column_start, column_end); + if (window_column_end > window_limit) window_column_end = window_limit; + pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line, rich_error->column_start, window_column_end); } const uint8_t *start = parser->start + parser->line_offsets.offsets[rich_error->line - parser->start_line]; @@ -308,9 +329,9 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ pm_buffer_append_string(buffer, format.blank_prefix, format.blank_prefix_length); size_t column = 0; - if (last_column_start >= TRUNCATE) { + if (window_column_start > 0) { pm_buffer_append_string(buffer, " ", 4); - column = last_column_start; + column = window_column_start; } while (column < rich_error->column_start) { @@ -335,7 +356,7 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ size_t char_width = pm_parser_encoding_char_width(parser, start + column, parser->end - (start + column)); column += (char_width == 0 ? 1 : char_width); - while (column < rich_error->column_end) { + while ((column < rich_error->column_end) && (column < window_column_end)) { pm_buffer_append_byte(buffer, '~'); size_t char_width = pm_parser_encoding_char_width(parser, start + column, parser->end - (start + column)); diff --git a/test/prism/api/raise_error_test.rb b/test/prism/api/raise_error_test.rb index 6ca12fa5e2..2d4a08fc8a 100644 --- a/test/prism/api/raise_error_test.rb +++ b/test/prism/api/raise_error_test.rb @@ -50,6 +50,23 @@ def test_syntax_error_color assert_include error.message, "\e[1;31m" end + def test_syntax_error_many_on_one_line + # Every comma is its own error. Errors on a line are displayed in windows + # of bounded width, so the message grows linearly with the input. + source = "," * 4096 + error = assert_raise(SyntaxError) { Prism.parse(source, raise_error: :plain) } + assert_operator error.message.bytesize, :<, source.bytesize * 200 + end + + def test_syntax_error_far_apart_on_one_line + # The errors at either end of the line are displayed in separate windows, + # so the whitespace between them is not repeated beneath the line. + source = "x#{" " * 100_000},," + error = assert_raise(SyntaxError) { Prism.parse(source, raise_error: :plain) } + assert_operator error.message.bytesize, :<, source.bytesize + assert_equal 2, error.message.scan("> 1 |").size + end + def test_syntax_error_not_utf8 error = assert_raise(SyntaxError) { Prism.parse("foo = 1 + \xFF\xFE\n".b, raise_error: :plain, filepath: "bin.rb") } assert_include error.message, "invalid multibyte character" From 0dac8f2bfc1e9eacc432c224b3b15da70db202f7 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 29 Oct 2026 11:11:29 -0400 Subject: [PATCH 2/5] Check UTF-8 quality only once in error formatting Instead of checking each line individually, sort all of the errors and do a single walk over the source to figure out if it is valid UTF-8 to determine how to format the errors. --- src/errors_format.c | 142 ++++++++++++++++++++--------- test/prism/api/raise_error_test.rb | 10 ++ 2 files changed, 111 insertions(+), 41 deletions(-) diff --git a/src/errors_format.c b/src/errors_format.c index 0785d90d46..616e7d46d1 100644 --- a/src/errors_format.c +++ b/src/errors_format.c @@ -15,19 +15,15 @@ #include /** - * Check if the source touched by the given location is valid UTF-8. The - * location represents the location of the error, but the slice of the source - * that will be displayed includes the content of all of the lines that the - * error touches, so we need to check those parts as well. + * Check if the source in the given range of lines is valid UTF-8. The indices + * are zero-based indices into the line offsets list, and the range is + * inclusive on both ends. */ static bool -location_is_utf8(const pm_parser_t *parser, const pm_location_t *location) { +lines_are_utf8(const pm_parser_t *parser, size_t start_index, size_t end_index) { const pm_line_offset_list_t *line_offsets = &parser->line_offsets; - const size_t start_line = (size_t) pm_line_offset_list_line_column(line_offsets, location->start, 1).line; - const size_t end_line = (size_t) pm_line_offset_list_line_column(line_offsets, location->start + location->length, 1).line; - - const uint8_t *cursor = parser->start + line_offsets->offsets[start_line - 1]; - const uint8_t *end = (end_line == line_offsets->size) ? parser->end : (parser->start + line_offsets->offsets[end_line]); + const uint8_t *cursor = parser->start + line_offsets->offsets[start_index]; + const uint8_t *end = (end_index + 1 == line_offsets->size) ? parser->end : (parser->start + line_offsets->offsets[end_index + 1]); while (cursor < end) { size_t width = pm_encoding_utf_8_char_width(cursor, end - cursor); @@ -41,7 +37,7 @@ location_is_utf8(const pm_parser_t *parser, const pm_location_t *location) { typedef struct { pm_diagnostic_t *diagnostic; - int32_t line; + int32_t line_start; uint32_t column_start; uint32_t column_end; size_t idx; @@ -52,8 +48,8 @@ pm_rich_error_compare(const void *a, const void *b) { const pm_rich_error_t *left = (const pm_rich_error_t *) a; const pm_rich_error_t *right = (const pm_rich_error_t *) b; - if (left->line != right->line) { - return (left->line < right->line) ? -1 : 1; + if (left->line_start != right->line_start) { + return (left->line_start < right->line_start) ? -1 : 1; } if (left->column_start != right->column_start) { @@ -67,6 +63,71 @@ pm_rich_error_compare(const void *a, const void *b) { return 0; } +/** + * Return the number of the last line of the source that can be displayed. When + * the source ends with a newline, the empty line after it is not displayed. + */ +static int32_t +pm_rich_errors_final_line(const pm_parser_t *parser) { + const pm_line_offset_list_t *line_offsets = &parser->line_offsets; + int32_t final_line = parser->start_line + ((int32_t) line_offsets->size) - 1; + + if ((parser->start + line_offsets->offsets[line_offsets->size - 1]) == parser->end) { + final_line--; + } + + return final_line; +} + +/** + * Find the range of lines that may be displayed around an error on the given + * line: up to two lines of context on either side of it, limited to the lines + * in the source. The error's own line is always included, even when it is the + * empty line after a trailing newline. + */ +static void +pm_rich_error_context(int32_t line, int32_t start_line, int32_t final_line, int32_t *first, int32_t *last) { + *first = (line - start_line > 2) ? (line - 2) : start_line; + *last = (final_line - line > 2) ? (line + 2) : final_line; + if (*last < line) *last = line; +} + +/** + * Check if the source that will be displayed for the given rich errors is + * valid UTF-8. That is each error's line and the lines of context around it. + * The errors must be sorted by their line. Each line is checked at most once, + * so that many errors on the same long line cost time proportional to the + * line, not to the line times the number of errors. + */ +static bool +rich_errors_are_utf8(const pm_parser_t *parser, size_t nerrors, const pm_rich_error_t *rich_errors) { + int32_t final_line = pm_rich_errors_final_line(parser); + + /* Every line from the first line of context of the most recently visited + * error up to (but not including) this index has already been checked. + * Because the errors are sorted, the first line of context never + * decreases, so any later error only needs the lines from this index + * onward. */ + size_t checked_index = 0; + + for (size_t idx = 0; idx < nerrors; idx++) { + int32_t first_line; + int32_t last_line; + pm_rich_error_context(rich_errors[idx].line_start, parser->start_line, final_line, &first_line, &last_line); + + size_t start_index = (size_t) (first_line - parser->start_line); + size_t end_index = (size_t) (last_line - parser->start_line); + + if (start_index < checked_index) start_index = checked_index; + if (start_index > end_index) continue; + + if (!lines_are_utf8(parser, start_index, end_index)) return false; + checked_index = end_index + 1; + } + + return true; +} + #define COLOR_BOLD "\033[1m" #define COLOR_GRAY "\033[2m" #define COLOR_RED "\033[1;31m" @@ -242,7 +303,7 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const char static void pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_format_type_t format_type, size_t nerrors, pm_rich_error_t *rich_errors, bool inline_messages) { pm_error_line_format_t format; - pm_error_line_format_init(&format, format_type, rich_errors[0].line, rich_errors[nerrors - 1].line); + pm_error_line_format_init(&format, format_type, rich_errors[0].line_start, rich_errors[nerrors - 1].line_start); /* We are going to iterate through every error in our error list and display * it. While we are iterating, we will display some padding lines of the @@ -265,18 +326,18 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ /* Here we determine how many lines of padding of the source to display, * based on the difference from the last line that was displayed. */ - if (rich_error->line - last_line > 1) { - if (rich_error->line - last_line > 2) { - if ((idx != 0) && (rich_error->line - last_line > 3)) { + if (rich_error->line_start - last_line > 1) { + if (rich_error->line_start - last_line > 2) { + if ((idx != 0) && (rich_error->line_start - last_line > 3)) { pm_buffer_append_string(buffer, format.divider, format.divider_length); } pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line - 2, 0, 0); + pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line_start - 2, 0, 0); } pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line - 1, 0, 0); + pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line_start - 1, 0, 0); } /* If this error is not in the current window (because it is the first @@ -305,15 +366,15 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ window_column_end = rich_error->column_end; for (window_end_idx = idx + 1; window_end_idx < nerrors; window_end_idx++) { const pm_rich_error_t *next_error = &rich_errors[window_end_idx]; - if ((next_error->line != rich_error->line) || (next_error->column_start >= window_limit)) break; + if ((next_error->line_start != rich_error->line_start) || (next_error->column_start >= window_limit)) break; if (next_error->column_end > window_column_end) window_column_end = next_error->column_end; } if (window_column_end > window_limit) window_column_end = window_limit; - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line, rich_error->column_start, window_column_end); + pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line_start, rich_error->column_start, window_column_end); } - const uint8_t *start = parser->start + parser->line_offsets.offsets[rich_error->line - parser->start_line]; + const uint8_t *start = parser->start + parser->line_offsets.offsets[rich_error->line_start - parser->start_line]; if (start == parser->end) pm_buffer_append_byte(buffer, '\n'); /* Now we will display the actual error message. We will do this by @@ -378,7 +439,7 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ /* Here we determine how many lines of padding to display after the * diagnostic, depending on where the next diagnostic is in source. */ - last_line = rich_error->line; + last_line = rich_error->line_start; int32_t next_line; if (idx == nerrors - 1) { @@ -391,7 +452,7 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ next_line--; } } else { - next_line = rich_errors[idx + 1].line; + next_line = rich_errors[idx + 1].line_start; } if (next_line - last_line > 1) { @@ -424,7 +485,7 @@ pm_rich_error_init(const pm_parser_t *parser, pm_rich_error_t *rich_error, pm_di *rich_error = (pm_rich_error_t) { .diagnostic = diagnostic, - .line = start_line_column.line, + .line_start = start_line_column.line, .column_start = start_line_column.column, .column_end = column_end }; @@ -444,7 +505,6 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma pm_rich_error_t *rich_errors = xmalloc(rich_errors_size); if (rich_errors == NULL) PRISM_OOM(); - bool is_utf8 = true; size_t rich_errors_idx = 0; for (pm_diagnostic_t *diagnostic = (pm_diagnostic_t *) parser->error_list.head; diagnostic != NULL; diagnostic = (pm_diagnostic_t *) diagnostic->node.next) { @@ -452,10 +512,6 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma switch (pm_diagnostic_error_level(diagnostic)) { case PM_ERROR_LEVEL_SYNTAX: { - if (is_utf8 && !location_is_utf8(parser, &location)) { - is_utf8 = false; - } - pm_rich_error_init(parser, &rich_errors[rich_errors_idx], diagnostic, &location); rich_errors[rich_errors_idx].idx = rich_errors_idx; rich_errors_idx++; @@ -474,9 +530,10 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma pm_diagnostic_message(diagnostic) ); - if (location_is_utf8(parser, &location)) { + pm_rich_error_init(parser, rich_errors, diagnostic, &location); + + if (rich_errors_are_utf8(parser, 1, rich_errors)) { pm_buffer_append_byte(buffer, '\n'); - pm_rich_error_init(parser, rich_errors, diagnostic, &location); pm_rich_errors_format(parser, buffer, format_type, 1, rich_errors, false); } @@ -504,23 +561,26 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma "%.*s:%" PRIi32 ": syntax error%s found\n", filepath_length, filepath, - rich_errors[0].line, + rich_errors[0].line_start, (nerrors > 1) ? "s" : "" ); - if (is_utf8) { + /* Sort the errors so that they are in the order that they appear in the + * source. This is the order they are displayed in, and it also lets the + * UTF-8 check visit each line only once. */ + qsort(rich_errors, nerrors, sizeof(pm_rich_error_t), pm_rich_error_compare); + + if (rich_errors_are_utf8(parser, nerrors, rich_errors)) { /* In here, we have all of the rich diagnostic information and we know - * the snippets are valid UTF-8. We will sort the errors so that they - * are displayed in the order that they appear in the source, and then - * format them all together with the source code snippets. */ - qsort(rich_errors, nerrors, sizeof(pm_rich_error_t), pm_rich_error_compare); + * the snippets are valid UTF-8, so we format them all together with + * the source code snippets. */ pm_rich_errors_format(parser, buffer, format_type, nerrors, rich_errors, true); } else { /* In here, we have content in the source that is not valid UTF-8. In * that case, we do not want to attempt to format the source code * snippets because we do not know how the terminal will handle it. The - * diagnostics are displayed in the order that they were generated, - * which is the order of the array before it has been sorted. */ + * diagnostics are displayed in the order that they appear in the + * source, as they are when the snippets are displayed. */ for (size_t idx = 0; idx < nerrors; idx++) { pm_rich_error_t *rich_error = &rich_errors[idx]; @@ -530,7 +590,7 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma "%.*s:%" PRIi32 ": %s", filepath_length, filepath, - rich_error->line, + rich_error->line_start, pm_diagnostic_message(rich_error->diagnostic) ); } diff --git a/test/prism/api/raise_error_test.rb b/test/prism/api/raise_error_test.rb index 2d4a08fc8a..11336bf871 100644 --- a/test/prism/api/raise_error_test.rb +++ b/test/prism/api/raise_error_test.rb @@ -67,6 +67,16 @@ def test_syntax_error_far_apart_on_one_line assert_equal 2, error.message.scan("> 1 |").size end + def test_syntax_error_not_utf8_in_context + # A line of context is displayed, so it must be valid UTF-8 for the + # snippet to be displayed, even though it has no error of its own. + source = "# caf\xE9\n1 2\n".b.force_encoding(Encoding::UTF_8) + error = assert_raise(SyntaxError) { Prism.parse(source, raise_error: :plain) } + + assert_predicate error.message, :valid_encoding? + assert_not_include error.message, "> 2 |" + end + def test_syntax_error_not_utf8 error = assert_raise(SyntaxError) { Prism.parse("foo = 1 + \xFF\xFE\n".b, raise_error: :plain, filepath: "bin.rb") } assert_include error.message, "invalid multibyte character" From 12608c7e655b6a4f3446c0eebe60ea10b4e5ff67 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 29 Oct 2026 11:32:09 -0400 Subject: [PATCH 3/5] Error formatting gutters Do not hard-code the potential values for gutters, instead build them on the fly based on what is actually found in the range of the errors. --- include/prism/internal/buffer.h | 10 ++ src/buffer.c | 11 ++ src/errors_format.c | 226 +++++++++++++++-------------- test/prism/api/raise_error_test.rb | 22 +++ 4 files changed, 158 insertions(+), 111 deletions(-) diff --git a/include/prism/internal/buffer.h b/include/prism/internal/buffer.h index e7c4001846..3bcc2f0786 100644 --- a/include/prism/internal/buffer.h +++ b/include/prism/internal/buffer.h @@ -31,12 +31,22 @@ void pm_buffer_cleanup(pm_buffer_t *buffer); /* Append the given amount of space as zeroes to the buffer. */ void pm_buffer_append_zeroes(pm_buffer_t *buffer, size_t length); +/* Append the given byte to the buffer the given number of times. */ +void pm_buffer_append_repeated(pm_buffer_t *buffer, uint8_t value, size_t length); + /* Append a formatted string to the buffer. */ void pm_buffer_append_format(pm_buffer_t *buffer, const char *format, ...) PRISM_ATTRIBUTE_FORMAT(2, 3); /* Append a string to the buffer. */ void pm_buffer_append_string(pm_buffer_t *buffer, const char *value, size_t length); +/* + * Append a string literal to the buffer, with its length computed at compile + * time. Concatenating with "" means anything other than a string literal fails + * to compile, instead of having its pointer size taken as its length. + */ +#define PM_BUFFER_APPEND_LITERAL(buffer, literal) pm_buffer_append_string((buffer), "" literal, sizeof(literal) - 1) + /* Append a list of bytes to the buffer. */ void pm_buffer_append_bytes(pm_buffer_t *buffer, const uint8_t *value, size_t length); diff --git a/src/buffer.c b/src/buffer.c index 849d3f2eaa..c9f891ecd1 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -104,6 +104,17 @@ pm_buffer_append_zeroes(pm_buffer_t *buffer, size_t length) { } } +/** + * Append the given byte to the buffer the given number of times. + */ +void +pm_buffer_append_repeated(pm_buffer_t *buffer, uint8_t value, size_t length) { + size_t cursor = buffer->length; + if (pm_buffer_append_length(buffer, length)) { + memset(buffer->value + cursor, value, length); + } +} + /** * Append a formatted string to the buffer. */ diff --git a/src/errors_format.c b/src/errors_format.c index 616e7d46d1..05ba1922be 100644 --- a/src/errors_format.c +++ b/src/errors_format.c @@ -140,104 +140,108 @@ rich_errors_are_utf8(const pm_parser_t *parser, size_t nerrors, const pm_rich_er #define WINDOW 80 typedef struct { - const char *number_prefix; - const char *blank_prefix; - const char *divider; - size_t blank_prefix_length; - size_t divider_length; + /** The type of formatting to use when formatting the errors. */ + pm_errors_format_type_t type; + + /** + * The number of columns taken up by the widest line number that will be + * displayed, including the sign of a negative line number. The widest + * possible is the 11 columns of -2147483648. + */ + uint8_t width; } pm_error_line_format_t; +/** + * Write the decimal digits of the given line number, including the sign if it + * is negative, so that they end at the end of the given buffer. The buffer is + * large enough for the widest possible line number, -2147483648. Returns the + * number of characters written. + */ +static uint8_t +pm_error_line_digits(int32_t line, char digits[11]) { + /* The magnitude is computed in unsigned arithmetic so that negating the + * minimum value of a 32-bit integer is well-defined. */ + uint32_t magnitude = (line < 0) ? (0U - (uint32_t) line) : (uint32_t) line; + uint8_t length = 0; + + do { + digits[10 - length++] = (char) ('0' + (magnitude % 10)); + magnitude /= 10; + } while (magnitude > 0); + + if (line < 0) digits[10 - length++] = '-'; + return length; +} + +/** + * Append the given line number to the buffer, right aligned within the given + * width. + */ +static void +pm_error_append_line_number(pm_buffer_t *buffer, int32_t line, uint8_t width) { + char digits[11]; + uint8_t length = pm_error_line_digits(line, digits); + + if (width > length) pm_buffer_append_repeated(buffer, ' ', (size_t) (width - length)); + pm_buffer_append_string(buffer, digits + sizeof(digits) - length, length); +} + /** * Initialize the given line format struct based on whether or not to highlight - * the line and the first and last line numbers. + * the line and the first and last line numbers that will be displayed. Every + * line number in between is no wider than one of these two. */ static void pm_error_line_format_init(pm_error_line_format_t *format, pm_errors_format_type_t format_type, int32_t first_line, int32_t last_line) { - /* If we have a maximum line number that is negative, then we are going to - * use the absolute value for comparison but multiply by 10 to additionally - * have a column for the negative sign. */ - if (first_line < 0) first_line = (-first_line) * 10; - if (last_line < 0) last_line = (-last_line) * 10; - int32_t max_line = first_line > last_line ? first_line : last_line; - - if (max_line < 10) { - if (format_type > PM_ERRORS_FORMAT_PLAIN) { - *format = (pm_error_line_format_t) { - .number_prefix = COLOR_GRAY "%1" PRIi32 " | " COLOR_RESET, - .blank_prefix = COLOR_GRAY " | " COLOR_RESET, - .divider = COLOR_GRAY " ~~~~~" COLOR_RESET "\n" - }; - } else { - *format = (pm_error_line_format_t) { - .number_prefix = "%1" PRIi32 " | ", - .blank_prefix = " | ", - .divider = " ~~~~~\n" - }; - } - } else if (max_line < 100) { - if (format_type > PM_ERRORS_FORMAT_PLAIN) { - *format = (pm_error_line_format_t) { - .number_prefix = COLOR_GRAY "%2" PRIi32 " | " COLOR_RESET, - .blank_prefix = COLOR_GRAY " | " COLOR_RESET, - .divider = COLOR_GRAY " ~~~~~~" COLOR_RESET "\n" - }; - } else { - *format = (pm_error_line_format_t) { - .number_prefix = "%2" PRIi32 " | ", - .blank_prefix = " | ", - .divider = " ~~~~~~\n" - }; - } - } else if (max_line < 1000) { - if (format_type > PM_ERRORS_FORMAT_PLAIN) { - *format = (pm_error_line_format_t) { - .number_prefix = COLOR_GRAY "%3" PRIi32 " | " COLOR_RESET, - .blank_prefix = COLOR_GRAY " | " COLOR_RESET, - .divider = COLOR_GRAY " ~~~~~~~" COLOR_RESET "\n" - }; - } else { - *format = (pm_error_line_format_t) { - .number_prefix = "%3" PRIi32 " | ", - .blank_prefix = " | ", - .divider = " ~~~~~~~\n" - }; - } - } else if (max_line < 10000) { - if (format_type > PM_ERRORS_FORMAT_PLAIN) { - *format = (pm_error_line_format_t) { - .number_prefix = COLOR_GRAY "%4" PRIi32 " | " COLOR_RESET, - .blank_prefix = COLOR_GRAY " | " COLOR_RESET, - .divider = COLOR_GRAY " ~~~~~~~~" COLOR_RESET "\n" - }; - } else { - *format = (pm_error_line_format_t) { - .number_prefix = "%4" PRIi32 " | ", - .blank_prefix = " | ", - .divider = " ~~~~~~~~\n" - }; - } - } else { - if (format_type > PM_ERRORS_FORMAT_PLAIN) { - *format = (pm_error_line_format_t) { - .number_prefix = COLOR_GRAY "%5" PRIi32 " | " COLOR_RESET, - .blank_prefix = COLOR_GRAY " | " COLOR_RESET, - .divider = COLOR_GRAY " ~~~~~~~~" COLOR_RESET "\n" - }; - } else { - *format = (pm_error_line_format_t) { - .number_prefix = "%5" PRIi32 " | ", - .blank_prefix = " | ", - .divider = " ~~~~~~~~\n" - }; - } - } + char digits[11]; + uint8_t first_width = pm_error_line_digits(first_line, digits); + uint8_t last_width = pm_error_line_digits(last_line, digits); - format->blank_prefix_length = strlen(format->blank_prefix); - format->divider_length = strlen(format->divider); + *format = (pm_error_line_format_t) { + .type = format_type, + .width = (first_width > last_width) ? first_width : last_width + }; } +/** + * Append the prefix for a line of source, which displays the line number right + * aligned within the width of the widest line number. + */ static void -pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const char *number_prefix, int32_t line, uint32_t column_start, uint32_t column_end) { +pm_error_line_format_number(const pm_error_line_format_t *format, pm_buffer_t *buffer, int32_t line) { + if (format->type > PM_ERRORS_FORMAT_PLAIN) PM_BUFFER_APPEND_LITERAL(buffer, COLOR_GRAY); + pm_error_append_line_number(buffer, line, format->width); + PM_BUFFER_APPEND_LITERAL(buffer, " | "); + if (format->type > PM_ERRORS_FORMAT_PLAIN) PM_BUFFER_APPEND_LITERAL(buffer, COLOR_RESET); +} + +/** + * Append the prefix for a line beneath the source that displays an error, which + * is blank where the line number would be so that the columns line up. + */ +static void +pm_error_line_format_blank(const pm_error_line_format_t *format, pm_buffer_t *buffer) { + if (format->type > PM_ERRORS_FORMAT_PLAIN) PM_BUFFER_APPEND_LITERAL(buffer, COLOR_GRAY); + pm_buffer_append_repeated(buffer, ' ', (size_t) format->width + 1); + PM_BUFFER_APPEND_LITERAL(buffer, "| "); + if (format->type > PM_ERRORS_FORMAT_PLAIN) PM_BUFFER_APPEND_LITERAL(buffer, COLOR_RESET); +} + +/** + * Append the divider that marks lines of source that are skipped between two + * displayed snippets. It extends just past the line number prefix. + */ +static void +pm_error_line_format_divider(const pm_error_line_format_t *format, pm_buffer_t *buffer) { + if (format->type > PM_ERRORS_FORMAT_PLAIN) PM_BUFFER_APPEND_LITERAL(buffer, COLOR_GRAY); + PM_BUFFER_APPEND_LITERAL(buffer, " "); + pm_buffer_append_repeated(buffer, '~', (size_t) format->width + 4); + if (format->type > PM_ERRORS_FORMAT_PLAIN) PM_BUFFER_APPEND_LITERAL(buffer, COLOR_RESET); + pm_buffer_append_byte(buffer, '\n'); +} + +static void +pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const pm_error_line_format_t *format, int32_t line, uint32_t column_start, uint32_t column_end) { int32_t line_delta = line - parser->start_line; assert(line_delta >= 0); @@ -253,7 +257,7 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const char end = parser->start + parser->line_offsets.offsets[index + 1]; } - pm_buffer_append_format(buffer, number_prefix, line); + pm_error_line_format_number(format, buffer, line); /* Here we determine if we should truncate the start of the line. The * column is the start of a diagnostic, so it is on a character boundary. */ @@ -302,8 +306,19 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const char static void pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_format_type_t format_type, size_t nerrors, pm_rich_error_t *rich_errors, bool inline_messages) { + int32_t final_line = pm_rich_errors_final_line(parser); + + /* The line numbers that are displayed range from the first line of context + * of the first error to the last line of context of the last error. */ + int32_t first_displayed_line; + int32_t last_displayed_line; + int32_t unused_line; + pm_rich_error_context(rich_errors[0].line_start, parser->start_line, final_line, &first_displayed_line, &unused_line); + pm_rich_error_context(rich_errors[nerrors - 1].line_start, parser->start_line, final_line, &unused_line, &last_displayed_line); + pm_error_line_format_t format; - pm_error_line_format_init(&format, format_type, rich_errors[0].line_start, rich_errors[nerrors - 1].line_start); + pm_error_line_format_init(&format, format_type, first_displayed_line, last_displayed_line); + /* We are going to iterate through every error in our error list and display * it. While we are iterating, we will display some padding lines of the @@ -329,15 +344,15 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ if (rich_error->line_start - last_line > 1) { if (rich_error->line_start - last_line > 2) { if ((idx != 0) && (rich_error->line_start - last_line > 3)) { - pm_buffer_append_string(buffer, format.divider, format.divider_length); + pm_error_line_format_divider(&format, buffer); } pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line_start - 2, 0, 0); + pm_error_format_line(parser, buffer, &format, rich_error->line_start - 2, 0, 0); } pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line_start - 1, 0, 0); + pm_error_format_line(parser, buffer, &format, rich_error->line_start - 1, 0, 0); } /* If this error is not in the current window (because it is the first @@ -371,7 +386,7 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ } if (window_column_end > window_limit) window_column_end = window_limit; - pm_error_format_line(parser, buffer, format.number_prefix, rich_error->line_start, rich_error->column_start, window_column_end); + pm_error_format_line(parser, buffer, &format, rich_error->line_start, rich_error->column_start, window_column_end); } const uint8_t *start = parser->start + parser->line_offsets.offsets[rich_error->line_start - parser->start_line]; @@ -387,7 +402,7 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ * languages or emoji, this means it can be thrown off pretty badly. We * will need to solve this eventually. */ pm_buffer_append_string(buffer, " ", 2); - pm_buffer_append_string(buffer, format.blank_prefix, format.blank_prefix_length); + pm_error_line_format_blank(&format, buffer); size_t column = 0; if (window_column_start > 0) { @@ -438,31 +453,20 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ pm_buffer_append_byte(buffer, '\n'); /* Here we determine how many lines of padding to display after the - * diagnostic, depending on where the next diagnostic is in source. */ + * diagnostic, depending on where the next diagnostic is in source. The + * line after the last error is one past the last line that can be + * displayed. */ last_line = rich_error->line_start; - int32_t next_line; - - if (idx == nerrors - 1) { - next_line = ((int32_t) parser->line_offsets.size) + parser->start_line; - - /* If the file ends with a newline, subtract one from our - * "next_line" so that we do not output an extra line at the end of - * the file. */ - if ((parser->start + parser->line_offsets.offsets[parser->line_offsets.size - 1]) == parser->end) { - next_line--; - } - } else { - next_line = rich_errors[idx + 1].line_start; - } + int32_t next_line = (idx == nerrors - 1) ? (final_line + 1) : rich_errors[idx + 1].line_start; if (next_line - last_line > 1) { pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, format.number_prefix, ++last_line, 0, 0); + pm_error_format_line(parser, buffer, &format, ++last_line, 0, 0); } if (next_line - last_line > 1) { pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, format.number_prefix, ++last_line, 0, 0); + pm_error_format_line(parser, buffer, &format, ++last_line, 0, 0); } } } diff --git a/test/prism/api/raise_error_test.rb b/test/prism/api/raise_error_test.rb index 11336bf871..0280ea1844 100644 --- a/test/prism/api/raise_error_test.rb +++ b/test/prism/api/raise_error_test.rb @@ -67,6 +67,28 @@ def test_syntax_error_far_apart_on_one_line assert_equal 2, error.message.scan("> 1 |").size end + def test_syntax_error_wide_line_numbers + # The gutter is as wide as the widest line number displayed, so the + # markers beneath the line stay aligned with the source. + error = assert_raise(SyntaxError) { Prism.parse("1 2", raise_error: :plain, line: 123_456) } + source_line, marker_line = error.message.lines.grep(/\|/).first(2) + + assert_equal "> 123456 | 1 2\n", source_line + assert_equal source_line.index("|"), marker_line.index("|") + assert_equal source_line.rindex("2"), marker_line.index("^") + end + + def test_syntax_error_context_line_numbers + # The gutter is as wide as the widest line number displayed, including the + # lines of context after the last error. + source = "x = 1\n" * 98 + "1 2\n" + "y = 2\n" * 2 + error = assert_raise(SyntaxError) { Prism.parse(source, raise_error: :plain) } + gutters = error.message.lines.grep(/\|/).map { |line| line.index("|") } + + assert_includes error.message, "101 | y = 2" + assert_equal [gutters.first], gutters.uniq + end + def test_syntax_error_not_utf8_in_context # A line of context is displayed, so it must be valid UTF-8 for the # snippet to be displayed, even though it has no error of its own. From aab49af94b125b26c6f22850253d7905dde53b05 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 29 Oct 2026 11:44:08 -0400 Subject: [PATCH 4/5] Cap context lines at window size in error formatting --- src/errors_format.c | 25 ++++++++++++++++--------- test/prism/api/raise_error_test.rb | 9 +++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/errors_format.c b/src/errors_format.c index 05ba1922be..736dcba675 100644 --- a/src/errors_format.c +++ b/src/errors_format.c @@ -136,7 +136,8 @@ rich_errors_are_utf8(const pm_parser_t *parser, size_t nerrors, const pm_rich_er /* The maximum number of columns of a line that errors are displayed beneath * before the line is displayed again. This must be at least TRUNCATE so that - * every window after the first on a line starts with a truncated line. */ + * every window after the first on a line starts with a truncated line. Lines + * of context are cut off at the same width. */ #define WINDOW 80 typedef struct { @@ -304,6 +305,16 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const pm_er } } +/** + * Display a line of context around an error, which is cut off at the same width + * as a window so that a long line does not dominate the output. + */ +static void +pm_error_format_context_line(const pm_parser_t *parser, pm_buffer_t *buffer, const pm_error_line_format_t *format, int32_t line) { + PM_BUFFER_APPEND_LITERAL(buffer, " "); + pm_error_format_line(parser, buffer, format, line, 0, WINDOW); +} + static void pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_format_type_t format_type, size_t nerrors, pm_rich_error_t *rich_errors, bool inline_messages) { int32_t final_line = pm_rich_errors_final_line(parser); @@ -347,12 +358,10 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ pm_error_line_format_divider(&format, buffer); } - pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, &format, rich_error->line_start - 2, 0, 0); + pm_error_format_context_line(parser, buffer, &format, rich_error->line_start - 2); } - pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, &format, rich_error->line_start - 1, 0, 0); + pm_error_format_context_line(parser, buffer, &format, rich_error->line_start - 1); } /* If this error is not in the current window (because it is the first @@ -460,13 +469,11 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ int32_t next_line = (idx == nerrors - 1) ? (final_line + 1) : rich_errors[idx + 1].line_start; if (next_line - last_line > 1) { - pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, &format, ++last_line, 0, 0); + pm_error_format_context_line(parser, buffer, &format, ++last_line); } if (next_line - last_line > 1) { - pm_buffer_append_string(buffer, " ", 2); - pm_error_format_line(parser, buffer, &format, ++last_line, 0, 0); + pm_error_format_context_line(parser, buffer, &format, ++last_line); } } } diff --git a/test/prism/api/raise_error_test.rb b/test/prism/api/raise_error_test.rb index 0280ea1844..8347fbea00 100644 --- a/test/prism/api/raise_error_test.rb +++ b/test/prism/api/raise_error_test.rb @@ -99,6 +99,15 @@ def test_syntax_error_not_utf8_in_context assert_not_include error.message, "> 2 |" end + def test_syntax_error_long_context_line + # Lines of context are cut off at the width of a window. + source = "x = #{"a" * 100_000}\n1 2\n" + error = assert_raise(SyntaxError) { Prism.parse(source, raise_error: :plain) } + + assert_operator error.message.bytesize, :<, 1000 + assert_match(/^ 1 \| x = a+ \.\.\.$/, error.message) + end + def test_syntax_error_not_utf8 error = assert_raise(SyntaxError) { Prism.parse("foo = 1 + \xFF\xFE\n".b, raise_error: :plain, filepath: "bin.rb") } assert_include error.message, "invalid multibyte character" From 740a4adc59dfe176cc40df70c154d7ad3acb5015 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 29 Oct 2026 23:02:18 -0400 Subject: [PATCH 5/5] Speed up error formatting --- src/errors_format.c | 117 +++++++++++++++++++++++--------------------- 1 file changed, 61 insertions(+), 56 deletions(-) diff --git a/src/errors_format.c b/src/errors_format.c index 736dcba675..645a997088 100644 --- a/src/errors_format.c +++ b/src/errors_format.c @@ -241,6 +241,35 @@ pm_error_line_format_divider(const pm_error_line_format_t *format, pm_buffer_t * pm_buffer_append_byte(buffer, '\n'); } +/** + * Advance the given column of the line that begins at start by whole + * characters until it reaches the given limit, and return the number of + * characters passed over. Each character is displayed in one column. A byte + * that does not begin a valid character, or that is past the end of the + * source, counts as one character. + */ +static size_t +pm_error_advance_columns(const pm_parser_t *parser, const uint8_t *start, size_t *column, size_t limit) { + size_t count = 0; + + while (*column < limit) { + const uint8_t *cursor = start + *column; + size_t char_width = 1; + + /* Every supported encoding is ASCII-compatible, so a byte below 0x80 + * at the start of a character is a character by itself. */ + if ((cursor < parser->end) && (*cursor >= 0x80)) { + char_width = pm_parser_encoding_char_width(parser, cursor, parser->end - cursor); + if (char_width == 0) char_width = 1; + } + + *column += char_width; + count++; + } + + return count; +} + static void pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const pm_error_line_format_t *format, int32_t line, uint32_t column_start, uint32_t column_end) { int32_t line_delta = line - parser->start_line; @@ -295,11 +324,11 @@ pm_error_format_line(const pm_parser_t *parser, pm_buffer_t *buffer, const pm_er truncate_end = true; } - if (truncate_start) pm_buffer_append_string(buffer, "... ", 4); + if (truncate_start) PM_BUFFER_APPEND_LITERAL(buffer, "... "); pm_buffer_append_string(buffer, (const char *) display_start, (size_t) (end - display_start)); if (truncate_end) { - pm_buffer_append_string(buffer, " ...\n", 5); + PM_BUFFER_APPEND_LITERAL(buffer, " ...\n"); } else if (end == parser->end && end > parser->start && end[-1] != '\n') { pm_buffer_append_byte(buffer, '\n'); } @@ -330,7 +359,6 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ pm_error_line_format_t format; pm_error_line_format_init(&format, format_type, first_displayed_line, last_displayed_line); - /* We are going to iterate through every error in our error list and display * it. While we are iterating, we will display some padding lines of the * source before the error to give some context. We will be careful not to @@ -370,13 +398,13 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ if (idx == window_end_idx) { switch (format_type) { case PM_ERRORS_FORMAT_PLAIN: - pm_buffer_append_string(buffer, "> ", 2); + PM_BUFFER_APPEND_LITERAL(buffer, "> "); break; case PM_ERRORS_FORMAT_STYLE: - pm_buffer_append_format(buffer, "%s", COLOR_BOLD "> " COLOR_RESET); + PM_BUFFER_APPEND_LITERAL(buffer, COLOR_BOLD "> " COLOR_RESET); break; case PM_ERRORS_FORMAT_COLOR: - pm_buffer_append_format(buffer, "%s", COLOR_RED "> " COLOR_RESET); + PM_BUFFER_APPEND_LITERAL(buffer, COLOR_RED "> " COLOR_RESET); break; } @@ -410,46 +438,37 @@ pm_rich_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_ * character when displayed in the terminal. For some east-asian * languages or emoji, this means it can be thrown off pretty badly. We * will need to solve this eventually. */ - pm_buffer_append_string(buffer, " ", 2); + PM_BUFFER_APPEND_LITERAL(buffer, " "); pm_error_line_format_blank(&format, buffer); size_t column = 0; if (window_column_start > 0) { - pm_buffer_append_string(buffer, " ", 4); + PM_BUFFER_APPEND_LITERAL(buffer, " "); column = window_column_start; } - while (column < rich_error->column_start) { - pm_buffer_append_byte(buffer, ' '); - - size_t char_width = pm_parser_encoding_char_width(parser, start + column, parser->end - (start + column)); - column += (char_width == 0 ? 1 : char_width); - } + pm_buffer_append_repeated(buffer, ' ', pm_error_advance_columns(parser, start, &column, rich_error->column_start)); switch (format_type) { case PM_ERRORS_FORMAT_PLAIN: pm_buffer_append_byte(buffer, '^'); break; case PM_ERRORS_FORMAT_STYLE: - pm_buffer_append_format(buffer, "%s^", COLOR_BOLD); + PM_BUFFER_APPEND_LITERAL(buffer, COLOR_BOLD "^"); break; case PM_ERRORS_FORMAT_COLOR: - pm_buffer_append_format(buffer, "%s^", COLOR_RED); + PM_BUFFER_APPEND_LITERAL(buffer, COLOR_RED "^"); break; } - size_t char_width = pm_parser_encoding_char_width(parser, start + column, parser->end - (start + column)); - column += (char_width == 0 ? 1 : char_width); - - while ((column < rich_error->column_end) && (column < window_column_end)) { - pm_buffer_append_byte(buffer, '~'); - - size_t char_width = pm_parser_encoding_char_width(parser, start + column, parser->end - (start + column)); - column += (char_width == 0 ? 1 : char_width); - } + /* The caret marks the first character of the error, and the tildes + * mark the rest of it, up to the end of the window. */ + pm_error_advance_columns(parser, start, &column, column + 1); + size_t tilde_limit = (rich_error->column_end < window_column_end) ? rich_error->column_end : window_column_end; + pm_buffer_append_repeated(buffer, '~', pm_error_advance_columns(parser, start, &column, tilde_limit)); if (format_type != PM_ERRORS_FORMAT_PLAIN) { - pm_buffer_append_format(buffer, "%s", COLOR_RESET); + PM_BUFFER_APPEND_LITERAL(buffer, COLOR_RESET); } if (inline_messages) { @@ -502,14 +521,24 @@ pm_rich_error_init(const pm_parser_t *parser, pm_rich_error_t *rich_error, pm_di }; } +/** + * Append a line that names the file and line of an error along with its + * message, which is how an error is displayed without a source snippet. + */ +static void +pm_error_append_summary(const pm_parser_t *parser, pm_buffer_t *buffer, int32_t line, const char *message) { + pm_buffer_append_string(buffer, (const char *) pm_string_source(&parser->filepath), pm_string_length(&parser->filepath)); + pm_buffer_append_byte(buffer, ':'); + pm_error_append_line_number(buffer, line, 0); + PM_BUFFER_APPEND_LITERAL(buffer, ": "); + pm_buffer_append_string(buffer, message, strlen(message)); +} + pm_error_level_t pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_format_type_t format_type) { size_t nerrors = parser->error_list.size; assert(nerrors > 0); - int filepath_length = (int) pm_string_length(&parser->filepath); - const char *filepath = (const char *) pm_string_source(&parser->filepath); - size_t rich_errors_size = nerrors * sizeof(pm_rich_error_t); if (rich_errors_size / sizeof(pm_rich_error_t) != nerrors) abort(); @@ -532,16 +561,8 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma /* If we get an argument error, we want to format it and return it * immediately. They should include a snippet of the source if * possible. */ - pm_buffer_append_format( - buffer, - "%.*s:%" PRIi32 ": %s", - filepath_length, - filepath, - pm_line_offset_list_line(&parser->line_offsets, location.start, parser->start_line), - pm_diagnostic_message(diagnostic) - ); - pm_rich_error_init(parser, rich_errors, diagnostic, &location); + pm_error_append_summary(parser, buffer, rich_errors[0].line_start, pm_diagnostic_message(diagnostic)); if (rich_errors_are_utf8(parser, 1, rich_errors)) { pm_buffer_append_byte(buffer, '\n'); @@ -567,14 +588,7 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma /* The header displays the line of the first error in parse order, which * is the first entry in the array because it has not been sorted yet. */ - pm_buffer_append_format( - buffer, - "%.*s:%" PRIi32 ": syntax error%s found\n", - filepath_length, - filepath, - rich_errors[0].line_start, - (nerrors > 1) ? "s" : "" - ); + pm_error_append_summary(parser, buffer, rich_errors[0].line_start, (nerrors > 1) ? "syntax errors found\n" : "syntax error found\n"); /* Sort the errors so that they are in the order that they appear in the * source. This is the order they are displayed in, and it also lets the @@ -593,17 +607,8 @@ pm_errors_format(const pm_parser_t *parser, pm_buffer_t *buffer, pm_errors_forma * diagnostics are displayed in the order that they appear in the * source, as they are when the snippets are displayed. */ for (size_t idx = 0; idx < nerrors; idx++) { - pm_rich_error_t *rich_error = &rich_errors[idx]; - if (idx > 0) pm_buffer_append_byte(buffer, '\n'); - pm_buffer_append_format( - buffer, - "%.*s:%" PRIi32 ": %s", - filepath_length, - filepath, - rich_error->line_start, - pm_diagnostic_message(rich_error->diagnostic) - ); + pm_error_append_summary(parser, buffer, rich_errors[idx].line_start, pm_diagnostic_message(rich_errors[idx].diagnostic)); } }