Re: [vim/vim] errorformat %r seems to imply %> (#2785)

2018-04-07 Fir de Conversatie Yegappan Lakshmanan
Hi,

On Sat, Apr 7, 2018 at 7:34 AM, Yegappan Lakshmanan  wrote:
> Hi,
>
> On Fri, Apr 6, 2018 at 10:13 PM, Jan Gosmann  
> wrote:
>> I can definitely do that, but might take me a few days.
>>
>> Also, I already took a look at the code myself and was considering moving
>> the restofline label to line 847. I suppose which of these is the proper fix
>> depends on whether it makes sense to use %> with the O/P/Q patterns. There
>> is also a line fields->valid = TRUE; that would then be executed. Not
>> entirely sure what the effect of that would be.
>>
>
> Your suggestion will also work. Some user may try to use %> with the
> O/P/Q patterns. So it is better to move the restofline label. The valid field
> is used to track whether the matched quickfix entry is a valid one (with a
> filename and line number) or not. As we are starting over to find a new match,
> it makes sense to set the field to TRUE again.
>

I am attaching a patch for this issue with a test for this case.

- Yegappan

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


multifile.diff
Description: Binary data


Re: [vim/vim] errorformat %r seems to imply %> (#2785)

2018-04-07 Fir de Conversatie Yegappan Lakshmanan
Hi,

On Fri, Apr 6, 2018 at 10:13 PM, Jan Gosmann  wrote:
> I can definitely do that, but might take me a few days.
>
> Also, I already took a look at the code myself and was considering moving
> the restofline label to line 847. I suppose which of these is the proper fix
> depends on whether it makes sense to use %> with the O/P/Q patterns. There
> is also a line fields->valid = TRUE; that would then be executed. Not
> entirely sure what the effect of that would be.
>

Your suggestion will also work. Some user may try to use %> with the
O/P/Q patterns. So it is better to move the restofline label. The valid field
is used to track whether the matched quickfix entry is a valid one (with a
filename and line number) or not. As we are starting over to find a new match,
it makes sense to set the field to TRUE again.

- Yegappan

>
> I'm not familiar with the
> Vim code base and even learned about the errorformat just in the last couple
> of days, but maybe it is helpful to have these things mentioned.
>

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [vim/vim] errorformat %r seems to imply %> (#2785)

2018-04-06 Fir de Conversatie Yegappan Lakshmanan
Hi,

On Thu, Apr 5, 2018 at 12:02 PM, Jan Gosmann  wrote:
> Given this error format:
>
> set efm=%trror\ l%l\ %m
> set efm+=%-P(%f%r
> set efm+=%-Q)%r
>
> and this error log:
>
> (file1
> Error l2 in file1
> (file2 (file3
> Error l4 in file3
> )
> Error l6 in file2
> (file4
> ))
> Error l9 in file 1 again
> (file5
> Error l11 in file5
> )(file6
> Error l13 in file6
> ))
>
> the quickfix list will be populated with the following
>
> file1|2 error| in file1
> file3|4 error| in file3
> file2|6 error| in file2
> file1|9 error| in file 1 again
> file5|11 error| in file5
> || (file6
> file1|13 error| in file6
>
> Note that the last error is wrongly reported in file1 instead of file6. The
> (file6 part is added to the quickfix list as non-valid entry, though it
> should be matched by set efm+=%-P(%f%r rule. It seems that %r implicitely
> assumes %>, i.e. restricts matches to the current and following patterns.
>
> This particular example could be fixed by switching the order of rule in
> efm, but doing so will result in other cases that do not work correctly
> (i.e. if a file is opened and closed on the same line like (fileA (fileB
> (fileC))).
>
> The documentation strongly suggest that it should be possible what I am
> doing here:
>
> Thus it is possible to parse even nested files like in the following line:
> {"file1" {"file2" error1} error2 {"file3" error3 {"file4" error4 error5}}}
>
> It also references the example of errorformat for LaTeX output which
> actually is what I am trying to parse in the end. That example does not work
> correctly for the lualatex output I am getting because of the issue
> described above.
>
> Version information
>
> VIM - Vi IMproved 8.0 (2016 Sep 12, compiled Apr  5 2018 14:36:32)
> Included patches: 1-402
> Compiled by Chakra
>
> I can add further information if requested or relevant.
>

Can you try the attached patch? It looks like when the support for "%>"
was added to 'errorformat' way back in 7.0, this was broken.

In my tests, this patch fixes the problem. Can you try this out in other
scenarios also to make sure this doesn't introduce any other problems?

Thanks,
Yegappan

diff --git a/src/quickfix.c b/src/quickfix.c
index 5c7abe78f..723a3e5d0 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -1118,6 +1118,7 @@ restofline:
{
STRMOVE(IObuff, skipwhite(tail));
qfl->qf_multiscan = TRUE;
+   fmt_ptr = fmt_first;
goto restofline;
}
}

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.