GNOME Bugzilla – Bug 482313
gregex: no way to tell why compilation failed
Last modified: 2007-12-03 18:58:46 UTC
g_regex_new does not return a reason why compilation failed. It ought to return the error code from pcre_compile2 somehow. Here's what g_regex_new does do: GError *tmp_error = g_error_new (G_REGEX_ERROR, G_REGEX_ERROR_COMPILE, _("Error while compiling regular " "expression %s at char %d: %s"), pattern, erroffset, errmsg); This is not useful. 1. erroffset is a byte offset, not a character count. 2. errmsg is untranslated and the final result is thus not something I can show to the user.
I will convert erroffset from byte offset to character offset before using it. I could add an array with known (and translatable) error codes, and use it instead of errmsg. If the error code is not known (because you are using a new version of pcre) then I could just use the untranslated string. Is it enough for your needs?
It would be enough for my current needs -- which is to be able to report errors to the user -- but I really think the actual error code should be embedded in the GError in roughly the same way that errno is embedded in errors from gfileutils.c And unknown error codes you can simply map to the generic "bad pattern" error.
Do you have a patch, Marco ?
Sorry, my plan was to fix this last week but university is taking too much time. After my last exam on Friday I will have time to work on this.
I committed the fix for using the character offset instead of the byte offset to trunk and the glib-2-14 branch.
Created attachment 99328 [details] [review] Return the (translatable) reason why compilation failed This patch adds new errors to GRegexError, this means that I can commit it only to trunk, isn't it? In this case I can backport the patch for glib 2.14 so we can have translated names without adding new symbols. Please check if the new error names have a decent and correct name.
And I have also to add "Since 2.16" in the documentation for the new errors.
I would like to see more of this in 2.14. Dual-language error messages is not something we can present to users. Also, please make sure the API says that new items might be added, i.e., that users should be prepared to map such errors to G_REGEX_ERROR_COMPILE or some such. - regex->extra = pcre_study (regex->pcre_re, 0, &errmsg); + regex->extra = pcre_study (regex->pcre_re, 0, (const gchar **)&errmsg); Isn't this an alias violation that causes gcc to complain?
For 2.14 I can just translate the message but without adding new error codes. No, gcc doesn't complain about alias violations.
Created attachment 99500 [details] [review] When compilation fails use a translatable error message and return the reason why it failed (for trunk) What do you mean exactly with "Also, please make sure the API..."?
Created attachment 99501 [details] [review] When compilation fails use a translatable error message (for glib-2-14) If you don't have any objection I'm going to commit this and the other patch tomorrow.
Created attachment 99502 [details] [review] When compilation fails use a translatable error message (for glib-2-14) If you don't have any objection I'm going to commit this and the other patch tomorrow.
> What do you mean exactly with "Also, please make sure the API..."? I mean that you should document that new error codes might be added (if PCRE is ungraded) and that the caller should be prepared for that. Patch looks good to me (visually -- untested).
Compilation of the regular expression failed with a generic error. I'd shorten that to Compilation of the regular expression failed. The "generic error" doesn't really add anything, and the shorter message is also more in sync with the one about optimization. Other than that, the patch looks ok to me, too. Since it adds new translatable strings, committing it to 2.14 will break the string freeze, thus you'll want to send a mail to gnome-i18n to notify translators about this.
Fixed in trunk and glib-2-14.