From 307bd77bc9fe91d4917772815bb6e378dc874631 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Wed, 15 Apr 2020 23:35:03 +0200 Subject: [PATCH 1/4] clang-format: work around vim/vim#5930 when positioning cursor 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: https://github.com/vim/vim/issues/5930 Similar workaround in first-party vim integration: https://github.com/llvm/llvm-project/commit/591be7ec500c151d9232366042e21c74e006292c --- autoload/codefmt/clangformat.vim | 39 +++++++++++++++++++++++--------- vroom/clangformat.vroom | 26 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/autoload/codefmt/clangformat.vim b/autoload/codefmt/clangformat.vim index 6835dd4..c9e08d9 100644 --- a/autoload/codefmt/clangformat.vim +++ b/autoload/codefmt/clangformat.vim @@ -120,32 +120,49 @@ function! codefmt#clangformat#GetFormatter() abort let l:cmd += ['-lines', l:startline . ':' . l:endline] endfor + let l:lines = getline(1, line('$')) + " Version 3.4 introduced support for cursor tracking " http://llvm.org/releases/3.4/tools/clang/docs/ClangFormat.html let l:supports_cursor = s:ClangFormatHasAtLeastVersion([3, 4]) if l:supports_cursor - " 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 let l:cmd += ['-cursor', string(l:cursor_pos)] endif - let l:input = join(getline(1, line('$')), "\n") + let l:input = join(l:lines, "\n") let l:result = maktaba#syscall#Create(l:cmd).WithStdin(l:input).Call() let l:formatted = split(l:result.stdout, "\n") - if !l:supports_cursor - call maktaba#buffer#Overwrite(1, line('$'), l:formatted[0:]) - else - call maktaba#buffer#Overwrite(1, line('$'), l:formatted[1:]) + if l:supports_cursor + " With -cursor, the first line is a JSON object. + let l:header = l:formatted[0] + let l:formatted = l:formatted[1:] + call maktaba#buffer#Overwrite(1, line('$'), l:formatted) try - let l:clang_format_output_json = maktaba#json#Parse(l:formatted[0]) - let l:new_cursor_pos = - \ maktaba#ensure#IsNumber(l:clang_format_output_json.Cursor) + 1 - execute 'goto' l:new_cursor_pos + let l:header_json = maktaba#json#Parse(l:header) + let l:offset = maktaba#ensure#IsNumber(l:header_json.Cursor) + " Compute line/col, avoid goto: https://github.com/vim/vim/issues/5930 + let l:new_line = 0 + for l:line in l:formatted + let l:offset_after_next = l:offset - len(l:line) - 1 + if l:offset_after_next < 0 + break + endif + let l:offset = l:offset_after_next + let l:new_line += 1 + endfor + call cursor(l:new_line + 1, l:offset + 1) catch call maktaba#error#Warn('Unable to parse clang-format cursor pos: %s', \ v:exception) endtry + else + call maktaba#buffer#Overwrite(1, line('$'), l:formatted) endif endfunction diff --git a/vroom/clangformat.vroom b/vroom/clangformat.vroom index 2fc012d..e5d7ea7 100644 --- a/vroom/clangformat.vroom +++ b/vroom/clangformat.vroom @@ -178,7 +178,33 @@ that clang-format has a version >= 3.4). :echomsg getline('.')[col('.')-1] ~ 5 +This should work when with textprops set in the file, despite various functions +like line2byte() and :goto being buggy in that case. +(See https://github.com/vim/vim/issues/5930 for bug details) + @clear + % int f() { + | int i=1; + | return 1234567890; } + :call prop_type_add('keyword', {}) + :call prop_add(1, 1, {'length': 3, 'type': 'keyword'}) + :call cursor(2, 10) + :echomsg getline('.')[col('.')-1] + ~ = + :FormatCode clang-format + ! clang-format -style file .* -cursor 23 .*2>.* + $ { "Cursor": 18 } + $ int f() { + $ int i = 1; + $ return 1234567890; + $ } + int f() { + int i = 1; + return 1234567890; + } + @end + :echomsg getline('.')[col('.')-1] + ~ = You might have wondered where the "-style file" above comes from. The clang-format tool accepts a "style" option to control the formatting style. By From 0cb9b7459a2d4b95ab51c208cfbc2cc7c47eb385 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Fri, 17 Apr 2020 21:45:24 +0200 Subject: [PATCH 2/4] Extract position <-> offset functions, simplify a little. --- autoload/codefmt/clangformat.vim | 50 ++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/autoload/codefmt/clangformat.vim b/autoload/codefmt/clangformat.vim index c9e08d9..2951fdd 100644 --- a/autoload/codefmt/clangformat.vim +++ b/autoload/codefmt/clangformat.vim @@ -55,6 +55,36 @@ function! s:ClangFormatHasAtLeastVersion(minimum_version) abort endfunction +" Inputs are 1-based (row, col) coordinates into lines. +" Returns the corresponding zero-based offset into lines->join("\n") +function! s:PositionToOffset(row, col, lines) abort + let l:offset = a:col - 1 + if a:row > 1 + for l:line in a:lines[0 : a:row - 2] + let l:offset += len(l:line) + 1 + endfor + endif + return l:offset +endfunction + + +" Input is zero-based offset into lines->join("\n") +" Returns the 1-based [row, col] coordinates into lines. +function! s:OffsetToPosition(offset, lines) abort + let l:lines_consumed = 0 + let l:chars_left = a:offset + for l:line in a:lines + let l:chars_after_next = l:chars_left - len(l:line) - 1 + if l:chars_after_next < 0 + break + endif + let l:chars_left = l:chars_after_next + let l:lines_consumed += 1 + endfor + return [l:lines_consumed + 1, l:chars_left + 1] +endfunction + + "" " @private " Invalidates the cached clang-format version. @@ -127,10 +157,7 @@ function! codefmt#clangformat#GetFormatter() abort let l:supports_cursor = s:ClangFormatHasAtLeastVersion([3, 4]) if l:supports_cursor " 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 + let l:cursor_pos = s:PositionToOffset(line('.'), col('.'), l:lines) let l:cmd += ['-cursor', string(l:cursor_pos)] endif @@ -140,23 +167,14 @@ function! codefmt#clangformat#GetFormatter() abort if l:supports_cursor " With -cursor, the first line is a JSON object. - let l:header = l:formatted[0] - let l:formatted = l:formatted[1:] + let l:header = remove(l:formatted, 0) call maktaba#buffer#Overwrite(1, line('$'), l:formatted) try let l:header_json = maktaba#json#Parse(l:header) let l:offset = maktaba#ensure#IsNumber(l:header_json.Cursor) " Compute line/col, avoid goto: https://github.com/vim/vim/issues/5930 - let l:new_line = 0 - for l:line in l:formatted - let l:offset_after_next = l:offset - len(l:line) - 1 - if l:offset_after_next < 0 - break - endif - let l:offset = l:offset_after_next - let l:new_line += 1 - endfor - call cursor(l:new_line + 1, l:offset + 1) + let [l:new_line, l:new_col] = s:OffsetToPosition(l:offset, l:formatted) + call cursor(l:new_line, l:new_col) catch call maktaba#error#Warn('Unable to parse clang-format cursor pos: %s', \ v:exception) From 00c93146c7bb920b27eab76b6a27e10abb681b9b Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Mon, 27 Apr 2020 12:44:14 +0200 Subject: [PATCH 3/4] Simplify OffsetToPosition loop Co-Authored-By: David Barnett --- autoload/codefmt/clangformat.vim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/autoload/codefmt/clangformat.vim b/autoload/codefmt/clangformat.vim index 2951fdd..81a043b 100644 --- a/autoload/codefmt/clangformat.vim +++ b/autoload/codefmt/clangformat.vim @@ -74,11 +74,11 @@ function! s:OffsetToPosition(offset, lines) abort let l:lines_consumed = 0 let l:chars_left = a:offset for l:line in a:lines - let l:chars_after_next = l:chars_left - len(l:line) - 1 - if l:chars_after_next < 0 + let l:line_len = len(l:line) + 1 + if l:chars_left < l:line_len break endif - let l:chars_left = l:chars_after_next + let l:chars_left -= l:line_len let l:lines_consumed += 1 endfor return [l:lines_consumed + 1, l:chars_left + 1] From eac7b921a77589f2017e9e5eef0cb38bd1d59d43 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Mon, 27 Apr 2020 13:30:54 +0200 Subject: [PATCH 4/4] Explain magic numbers --- autoload/codefmt/clangformat.vim | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/autoload/codefmt/clangformat.vim b/autoload/codefmt/clangformat.vim index 81a043b..536b968 100644 --- a/autoload/codefmt/clangformat.vim +++ b/autoload/codefmt/clangformat.vim @@ -58,10 +58,10 @@ endfunction " Inputs are 1-based (row, col) coordinates into lines. " Returns the corresponding zero-based offset into lines->join("\n") function! s:PositionToOffset(row, col, lines) abort - let l:offset = a:col - 1 + let l:offset = a:col - 1 " 1-based to 0-based if a:row > 1 - for l:line in a:lines[0 : a:row - 2] - let l:offset += len(l:line) + 1 + for l:line in a:lines[0 : a:row - 2] " 1-based to 0-based, exclude current + let l:offset += len(l:line) + 1 " +1 for newline endfor endif return l:offset @@ -74,14 +74,14 @@ function! s:OffsetToPosition(offset, lines) abort let l:lines_consumed = 0 let l:chars_left = a:offset for l:line in a:lines - let l:line_len = len(l:line) + 1 + let l:line_len = len(l:line) + 1 " +1 for newline if l:chars_left < l:line_len break endif let l:chars_left -= l:line_len let l:lines_consumed += 1 endfor - return [l:lines_consumed + 1, l:chars_left + 1] + return [l:lines_consumed + 1, l:chars_left + 1] " 0-based to 1-based endfunction