Yes. After looking at the error_regex persing, default compiler parsing and default message parsing, I was not satisfied with any of them, and so will do something Lex-like: propose a global unified solution. After a few more days to think about it, that is.
Yes, better to get it right slowly than wrong quickly :-)
Comment on the patch:
Documentation says:
17 + whether the first or second match is purely digits.
but (if I read the patch correctly) it actually only checks the first and decides line [col] file or file line [col]
"if (strtopos(match[1], line, TRUE) != NULL)" checks if the 2nd match is purely digits. line_idx is assigned before it for the match[line_idx] and possibly match[line_idx + 1] to be freed.
Ok.
In filetypes.c/filetypes_parse_error_message()
64 + pmatch[2] = pmatch[3];
I don't think its necessarily legal or safe to assign one match to another, an opaque structure like regmatch_t might have pointers to internal data and this might result in double free or cause other problems.
regex_t is opaque, while regmatch_t is documented in regexec(1) as having only rm_so and rm_eo. Even if there are more, a regmatch_t is never freed or anything.
Well you obviously get away with it because it isn't really opaque (to the compiler), but it is still bad practice to assign such structures unless it is documented safe.
The Geany convention is that symbols exported start with the prefix of the file they are exported from, and that words be separated by underlines, so strtopos should be utils_str_to_pos. That also makes it obvious that its a local function not a stdlib one.
Hmm, yes. My bad.
In strtopos() it would be better to use strtoul(), no need to check l
= 0 and the (int)l == l s/b l <= G_MAXINT since the return is a gint.
Indeed... (gint) l == l must be checked though, unsigned long and gint have different ranges.
Technically (gint)l == l is using implementation defined behavior, the standard says that the converting to a shorter signed type should give the same value if it fits, but if it doesn't fit, its implementation defined. On the other hand since the range of long is greater than or equal to the range in int which is what gint is, then the comparison to G_MAXINT is a correct way of doing it.
Now the two questions, mentioned in navqueue.c:
How about supporting line number 0? process_build_output_line() does, and I, too, remember some old compilers emiting line 0.
Sounds ok, line 0 == line 1?
Sure. My *cc at work emits line 0 sometimes...
I suppose thats as good as any for errors it doesn't have a line number for, so maybe we should use rand to pick a line number :-) oh, ok line 1.
[...]
On a 2nd thought, at this point we already switched to the file, so the filename was correct. Exiting navqueue_goto_line_col() with FALSE only prevents Enter and double click from focusing the editor, which is not very helpful. Addons/Tasks uses the return value, but doesn't seem like it really needs it. A void function will quickly solve the "what result to return if this or that argument is invalid or not entirely valid" question, but will have to wait until the release, so let's only add support for line = 0 for now.
Sounds fine.
Cheers Lex