clang-format: work around vim/vim#5930 when positioning cursor #145
Conversation
This bug (and a related predecessor) cause `:goto` and `line2bytes()` to do the wrong thing when vim textprops are used. This manifests as "cursor jumps around randomly after formatting" when using plugins that use textprops for diagnostics, highlighting etc. We happen to have all the code in an array when we need to do cursor coordinate conversions, so we do those in vimscript instead of querying vim. Without this patch, The newly added test fails on affected versions of vim including both vim master and debian's 8.1.2269 (in different ways). Vim bug tracker entry: vim/vim#5930 Similar workaround in first-party vim integration: llvm/llvm-project@591be7e
autoload/codefmt/clangformat.vim
Outdated
| " line2byte counts bytes from 1, and col counts from 1, so -2 | ||
| let l:cursor_pos = line2byte(line('.')) + col('.') - 2 | ||
| " Avoid line2byte: https://github.com/vim/vim/issues/5930 | ||
| let l:cursor_pos = col('.') - 1 | ||
| for l:i in range(1, line('.') - 1) | ||
| let l:cursor_pos += len(l:lines[l:i - 1]) + 1 | ||
| endfor |
There was a problem hiding this comment.
Consider refactoring this into an s:Func. Later it might make a nice helper in itself in maktaba#buffer#.
There was a problem hiding this comment.
Sure thing! I'm not sure how useful these are apart from as a workaround for this bug, I'll let you make the call on moving them.
|
Thanks for digging into this and fixing! |
|
Thanks for the review, and sorry about the hacky workarounds! |
Co-Authored-By: David Barnett <david@mumind.me>
|
@dbarnett Sorry for the long turnaround, and thanks for the review - this should be good to go. |
| % int f() {<CR> | ||
| | int i=1;<CR> | ||
| | return 1234567890; }<CR> | ||
| :call prop_type_add('keyword', {}) |
There was a problem hiding this comment.
Hmm, these make the vroom tests blow up on older versions of vim: https://travis-ci.org/github/google/vim-codefmt/jobs/680186570.
Can you change those to only conditionally add the props if vim supports them? And then IIUC the rest of the test should pass regardless on older vims, just won't be covering your workaround logic.
There was a problem hiding this comment.
Oops, sorry about that. Sent #150
Fixes clangformat.vroom failing against some versions of vim since #145 with `E117: Unknown function: prop_type_add`. Moves all the intricate cursor position checks from clangformat.vroom into a separate file clangformat-cursor.vroom so the flow of the basic testing scenarios makes makes more sense and is less brittle.
Fixes clangformat.vroom failing against some versions of vim since #145 with `E117: Unknown function: prop_type_add`. Moves all the intricate cursor position checks from clangformat.vroom into a separate file clangformat-cursor.vroom so the flow of the basic testing scenarios makes makes more sense and is less brittle.
This bug (and a related predecessor) cause
:gotoandline2bytes()to do the wrong thing when vim textprops are used.
This manifests as "cursor jumps around randomly after formatting" when using
plugins that use textprops for diagnostics, highlighting etc.
We happen to have all the code in an array when we need to do cursor coordinate
conversions, so we do those in vimscript instead of querying vim.
Without this patch, The newly added test fails on affected versions of vim
including both vim master and debian's 8.1.2269 (in different ways).
Vim bug tracker entry:
vim/vim#5930
Similar workaround in first-party vim integration:
llvm/llvm-project@591be7e