On Wed, 17 Aug 2011 11:33:40 +1000 Lex Trotman elextr@gmail.com wrote:
- IIUC lack of regexii (as you can tell I don't know what the
official plural of regex is :-) is because historically the hard coded approach existed first.
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.
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.
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.
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.
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...
How about leaving navqueue_goto_line_col() on pos == -1? According to Scintilla docs, it means that line - 1 is greater than the number of lines, and that's a wrong line # IMHO.
Agree.
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.
I'll send a "-b" version tomorrow.