Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TernRename in gvim, where the wrong buffer was selected #36

Closed
wants to merge 1 commit into from

Conversation

arian
Copy link
Contributor

@arian arian commented Aug 14, 2013

So the changes were made in the wrong buffer.

For me, the bufnr didn't correspond with the correct index in vim.buffers.
This only happened in gvim, in vim in the terminal it worked correctly.

Perhaps it's because of some plugin, I didn't try it separately.

So the changes were made in the wrong buffer.
@clausreinke
Copy link
Contributor

Don't have time to test this week, but didn't want to let this go unanswered: this is the first confirmed report we have of buffer indices not matching bufnr, thanks!

If I understand jparera's comment in #32 (comment) correctly, that issue is gone in vim 7.4, but we should still do something like the patch suggested here (just check that it works in 7.3 and 7.4, please). Some people have vast numbers of buffers open, which is why I instinctively avoided looping, but slow is better than wrong (and looping over a hundred or so buffers shouldn't really be slow).

@arian
Copy link
Contributor Author

arian commented Aug 15, 2013

Yes, seems I have the same problem as described in #32 and #24.

Perhaps you could have both bufnr and look that up in the vim.buffers, but if the buffer.name property does not equal, fall back to this loop to make it a bit faster in normal cases.

@jparera
Copy link

jparera commented Aug 15, 2013

I agree with @clausreinke, correctness is more important than performance.

Another workaround would be to detect the Vim version and register the looping version to TernRename only for vim <7.4. This way the overhead is done once instead of calling the method every time. To enable the loop, a variable could be defined.

if version < 740
        let g:tern_rename_enable_loop = 'yes'
else 
        let g:tern_rename_enable_loop = 'no'
endif

@clausreinke
Copy link
Contributor

@arian 's suggestion would cover both the 7.4 case and the common 7.3 case that index==bufnr (feature detection wins over version detection;-). Modulo error handling (if the buffer lists really are out of sync, there may not be a buffer at bufnr at all). Unless the python interface changes in 7.4 make a different code path necessary.

@jparera
Copy link

jparera commented Aug 15, 2013

According to the Vim changelog ( https://gist.github.com/mjhea0/6200588 ), previous versions expected iteration of vim.buffers and bufnr was not directly related with the internal buffer list. In 7.4 vim.buffers is dictionary which key is bufnr.

I have done some testing:

Vim 7.3:

  • IndexError: no such buffer is thrown if bufnr is directly used and its value is out of range of the internal index.

Vim 7.4:

  • ValueError: number must be greater than zero is thrown if zero is used as index.
  • KeyError is thrown if the buffer number does not exists.

If you use the modulo of bufnr you could get a KeyError in 7.4. I am not sure if you can do a feature detection mechanism and avoid error handling.

@marijnh
Copy link
Member

marijnh commented Aug 23, 2013

I've merged this (somewhat late, holiday got in the way) -- a linear scan over a set of buffers doesn't seem like it's likely to get problematic. If somebody wants to further refine it, feel free.

@marijnh marijnh closed this Aug 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants