[Geany-devel] msgwin line and column position

[Dimitar Zhekov]

On Wed, 17 Aug 2011 11:33:40 +1000
Lex Trotman <elextr at gmail.com> wrote:

> 3. 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.

-- 
E-gards: Jimmy