Skip to content

feat: improve formatting of error messages from C #2515

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
maelle wants to merge 1 commit into
base: main
Choose a base branch
from
+174 −81
Open

feat: improve formatting of error messages from C #2515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 19 additions & 4 deletions src/rinterface_extra.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2315,6 +2315,17 @@ static inline const char* maybe_add_punctuation(const char* msg, const char* pun
return is_punctuated(msg) ? "" : punctuation;
}

/* Strip vendor/cigraph/src/ prefix from file path */
static inline const char* simplify_file_path(const char* file) {
const char* prefix = "vendor/cigraph/src/";
size_t prefix_len = strlen(prefix);

if (strncmp(file, prefix, prefix_len) == 0) {
return file + prefix_len;
}
return file;
}
Comment on lines +2318 to +2327
Copy link
Member

@szhorvat szhorvat Jan 6, 2026

Choose a reason for hiding this comment

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

Suggested change
/* Strip vendor/cigraph/src/ prefix from file path */
static inline const char* simplify_file_path(const char* file) {
const char* prefix = "vendor/cigraph/src/";
size_t prefix_len = strlen(prefix);
if (strncmp(file, prefix, prefix_len) == 0) {
return file + prefix_len;
}
return file;
}
/* Strip vendor/cigraph/src/ prefix from file path. This prefix depends on the
* build procedure, namely on the directory that the compiler is invoked from. */
static inline const char* simplify_file_path(const char *file) {
const char prefix[] = "vendor/cigraph/src/";
const size_t prefix_len = sizeof(prefix) / sizeof(prefix[0]);
if (strncmp(file, prefix, prefix_len) == 0) {
return file + prefix_len;
}
return file;
}

You can get the length statically. It's good to have a comment about where this prefix comes from because it will change if changes are made to the build procedure. The reason it even appears (compared to the standard use of the C core) is the tweaked build procedure.


void Rx_igraph_fatal_handler(const char *reason, const char *file, int line) {
#ifdef IGRAPH_SANITIZER_AVAILABLE
__sanitizer_print_stack_trace();
Expand All @@ -2336,10 +2347,12 @@ void Rx_igraph_error_handler(const char *reason, const char *file,
* IGRAPH_FINALLY_FREE() can then clean it up. */

if (Rx_igraph_errors_count == 0 || !Rx_igraph_in_r_check) {
const char* simplified_path = simplify_file_path(file);
snprintf(Rx_igraph_error_reason, sizeof(Rx_igraph_error_reason),
"At %s:%i : %s%s %s", file, line, reason,
maybe_add_punctuation(reason, ","),
igraph_strerror(igraph_errno));
"%s%s %s\nSource: %s:%i", reason,
maybe_add_punctuation(reason, "."),
igraph_strerror(igraph_errno),
simplified_path, line);
Rx_igraph_error_reason[sizeof(Rx_igraph_error_reason) - 1] = 0;

// FIXME: This is a hack, we should replace all memory allocations in the
Expand All @@ -2356,8 +2369,10 @@ void Rx_igraph_error_handler(const char *reason, const char *file,

void Rx_igraph_warning_handler(const char *reason, const char *file, int line) {
if (Rx_igraph_warnings_count == 0) {
const char* simplified_path = simplify_file_path(file);
snprintf(Rx_igraph_warning_reason, sizeof(Rx_igraph_warning_reason),
"At %s:%i : %s%s", file, line, reason, maybe_add_punctuation(reason, "."));
"%s%s\nSource: %s:%i", reason, maybe_add_punctuation(reason, "."),
simplified_path, line);
Rx_igraph_warning_reason[sizeof(Rx_igraph_warning_reason) - 1] = 0;
}
Rx_igraph_warnings_count++;
Expand Down
Loading
Loading