[Geany-devel] msgwin line and column position

[Lex Trotman]

On 17 August 2011 04:11, Dimitar Zhekov <dimitar.zhekov at gmail.com> wrote:
> On Mon, 15 Aug 2011 10:19:26 +1000
> Lex Trotman <elextr at gmail.com> wrote:
>
>> Anyway if Dimitar wants to decode the column numbers in
>> msgwin_parse_compiler_error_line() and use them to set the cursor
>> thats fine, so long as column number is optional.
>
> Well you can always specify a two-match regular expression, such as
> "([^:]+):([0-9]+)" or "([^:]+):([0-9]+)[^A-Za-z]" for c++, and that
> will surely drop the column #, without the need for a special option.

That was the idea, I just wanted to ensure that a two match regex
doesn't cause errors.

>
>> Personally I wouldn't bother to add column numbers to the fallback
>> msgwindow.c/parse_compiler_error().
>
> Why not? There is only a single error_regex defined in the default
> configuration, everything else goes to the default parsing.

My reasons were:

1. save Dimitar some work (oh well too late :-)

2. from a quick inspection the only one of the defaults that would not
parse with regexen is HTML since it has line and col but no filename,
so there is no reason that columnated versions of the other compilers
shouldn't go to regexes instead of hard coded, hard coded is bad (tm)

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.

>
> --
>
> So here is the Compiler tab patch. Seeking to the exact column was a
> very nice extra when compiling Geany, even if I say so myself. :)
>

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]

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.

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.

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.

> navqueue_goto_line[_col]() are written this way to preserve
> compatibility, since there may be a release in September. For the same
> reason, the Messages tab will have to wait.
>
> 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?

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

Cheers
Lex