Hi, On Tue, Apr 12, 2016 at 9:59 AM, LCD 47 <[email protected]> wrote: > On 12 April 2016, Yegappan Lakshmanan <[email protected]> wrote: > > [...] >> > Can you please give an example when setting the cursor before an >> > autocmd would "break functionality that depends on these events"? >> > >> >> Consider for example, you have a BufReadPre autocommand that either >> uncompresses a file or performs a network operation to load the >> buffer. If these autocommands are delayed, then the buffer cannot be >> loaded and the location list cannot position the cursor. >> >> Note that selecting an entry from the location can trigger various >> autocommands depending on whether the buffer is already loaded in a >> window or hidden or not in memory. >> >> Also, depending on the 'switchbuf' setting, selecting an entry from >> the quickfix list can create a new window or a tab, jump to a tab or >> window. This will trigger additional autocommand events. You cannot >> delay autocommands in these cases (for example TabEnter/WinEnter). > > Right, this actually makes the case that patch 1640 is the wrong > fix, and also that the problem I'm pointing to has nothing to do with > patch 1592. >
I am not sure why you say the changes made for 1640 are wrong. The changes make sure that the location list entry is still valid after loading a buffer. > > 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? I am not sure how else this problem can be fixed. Without this change, Vim will reference freed memory and will crash. > > It also breaks autocmds for "example.txt" that modify the > contents of "example.txt", because line N from .ll might no longer be > the same after the autocmds. > The patch only checks whether the location list entry is valid. It doesn't check whether the contents of the buffer are changed or not. The autocmd can change the contents of the buffer. > > 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. > > 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). > > 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? - 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
