On 12 April 2016, Yegappan Lakshmanan <[email protected]> wrote:
> Hi,
>
> On Tue, Apr 12, 2016 at 9:59 AM, LCD 47 <[email protected]> wrote:
[...]
> >     Currently .ll when jumping to file "example.txt" line N goes
> > roughly like this:
> >
> > (1) load "example.txt"
> > (2) trigger autocmds for "example.txt"
> > (3) go to line N.
> >
> >     If the loclist changes between (2) and (3) there's trouble
> > because you're reading from free()d memory.  For this reason, your
> > patch 1640 adds a safeguard between (2) and (3), making sure the
> > loclist is still the same.
> >
>
> The patch makes sure that the location list entry (not the entire
> list) is still valid. Note that a autocmd can add entry to the current
> location list and modify entries which will not impact the current
> entry. But the autocmd cannot delete the current location list or load
> a new location list.
>
> >
> > This breaks autocmds for "example.txt" that override the current
> > loclist.
> >
>
> Do you have a real-world example of an existing plugin that breaks
> because of this change?

    As I pointed out in my previous message: syntastic and vim-go break
because of 1640.  They need to set a new loclist from an autocmd, that's
how they are supposed to work.

[...]
> >     I suggest swapping (2) and (3), that is, delaying autocmds for
> > "example.txt" until the jump is finished:
> >
> > (1') load "example.txt"
> > (2') go to line N
> > (3') trigger autocmds for "example.txt".
> >
> 
> As discussed in the earlier e-mails, triggering autocmd at this point
> is too late. To load the buffer and jump to the line, you need to
> trigger autocmds.

    We can keep running in circles if you like.  Please look at
quickfix.c lines 1857-1969 (in 1724), and point out which part would
change anything related to autocmds if run earlier.

> >     With this approach there is no need to care whether autocmds
> > change the current loclist or not, since the old loclist is no
> > longer needed, and can be safely discarded at that point.
> >
> >     The cursor also stand more chances to stay on the right line if
> > the autocmds change the contents of the buffer, since Vim can keep
> > track of it.
> >
>
> When the contents of a buffer is modified, Vim automatically updates
> the line numbers in the quickfix/location list entries (refer to the
> qf_mark_adjust() function).

    qf_mark_adjust() adjusts the number of lines in the buffer.  My
scenario is different:

(a) a loclist entry pointing to "example.txt" line 10
(b) an autocmd for "example.txt" that deletes lines 1-5.

    With your (existing) approach, "example.txt" is loaded and the
cursor is set to line 10.

    With mine, "example.txt" is loaded, the cursor is set to linew 10,
then lines 1-5 are deleted.  The cursor still stays on the right logical
line, because Vim keeps it there.

> >
> >     And finally, setting the cursor before running the autocmds is
> > highly unlikely to change the conditions that trigger said autocmds.
> > Reference: quickfix.c lines 1857-1969 in 1724.
> >
> >     Summary: 1640 was the wrong fix.  It breaks existing plugins,
> > such as syntastic and vim-go, which now have to find workarounds.
> >
>
> Are you guessing that this change may break those plugins or do you
> have a simplified example of how this change breaks those plugins?

    I do have a scenario that breaks syntastic.  It isn't simple,
because I already added a workaround that avoids the problem in most
other cases:

(1) set g:syntastic_always_populate_loc_list to 1
(2) set g:syntastic_auto_jump to 1
(3) set g:syntastic_cpp_check_header to 1
(4) set g:syntastic_check_on_open to 1
(5) set g:syntastic_cpp_checkers to ['gcc']
(6) set g:syntastic_cpp_compiler to 'clang++'
(7) open a C++ file, run the checker, and .ll to a problem in a header.

    Result: E926.

    Alternatively, just downgrade syntastic to some older release, run
any checker, and do a .ll .  More real world evidence:

https://github.com/scrooloose/syntastic/issues/1738
http://superuser.com/questions/1059194/vim-syntastic-not-jumping-to-the-error

    People elsewhere are discussing breaking cases for vim-go, but I'm
not using vim-go, so I won't comment on those.

    /lcd

-- 
-- 
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 [email protected].
For more options, visit https://groups.google.com/d/optout.

Raspunde prin e-mail lui