From 3aba91cf7124d3fa550b3b2aca83d9ba5024f485 Mon Sep 17 00:00:00 2001
From: Prabir Shrestha <mail@prabir.me>
Date: Wed, 1 Jan 2020 13:33:36 -0800
Subject: [PATCH] remove lsp#utils#to_col in favor of
 lsp#utils#position#_lsp_to_vim (#645)

* refactor to use lsp#utils#position#_lsp_to_vim
* remove lsp#utils#to_col in favor of lsp#utils#position#_lsp_to_vim
* fix doc
---
 autoload/lsp/omni.vim                        |  4 +--
 autoload/lsp/ui/vim.vim                      |  8 ++----
 autoload/lsp/ui/vim/diagnostics.vim          | 23 +++++-----------
 autoload/lsp/ui/vim/diagnostics/textprop.vim |  8 ++----
 autoload/lsp/ui/vim/highlights.vim           |  7 ++---
 autoload/lsp/ui/vim/references.vim           | 10 ++-----
 autoload/lsp/ui/vim/utils.vim                | 20 ++++----------
 autoload/lsp/utils.vim                       | 17 ------------
 autoload/lsp/utils/position.vim              | 23 ++++++++++++++--
 test/lsp/utils.vimspec                       | 28 --------------------
 10 files changed, 42 insertions(+), 106 deletions(-)

diff --git a/autoload/lsp/omni.vim b/autoload/lsp/omni.vim
index a8609daf..a4fd1771 100644
--- a/autoload/lsp/omni.vim
+++ b/autoload/lsp/omni.vim
@@ -456,9 +456,7 @@ endfunction
 function! s:get_cursor_pos_and_edit_length(text_edit) abort
     if !empty(a:text_edit)
         let l:start = a:text_edit['range']['start']
-        let l:line = l:start['line'] + 1
-        let l:char = l:start['character']
-        let l:col = lsp#utils#to_col('%', l:line, l:char)
+        let [l:line, l:col] = lsp#utils#position#_lsp_to_vim('%', l:start)
         let l:length = len(a:text_edit['newText'])
         let l:pos = [0, l:line, l:col, 0]
     else
diff --git a/autoload/lsp/ui/vim.vim b/autoload/lsp/ui/vim.vim
index 0786d443..120d9df1 100644
--- a/autoload/lsp/ui/vim.vim
+++ b/autoload/lsp/ui/vim.vim
@@ -542,12 +542,8 @@ function! s:handle_rename_prepare(server, last_req_id, type, data) abort
 
     let l:range = a:data['response']['result']
     let l:lines = getline(1, '$')
-    let l:start_line = l:range['start']['line'] + 1
-    let l:start_char = l:range['start']['character']
-    let l:start_col = lsp#utils#to_col('%', l:start_line, l:start_char)
-    let l:end_line = l:range['end']['line'] + 1
-    let l:end_char = l:range['end']['character']
-    let l:end_col = lsp#utils#to_col('%', l:end_line, l:end_char)
+    let [l:start_line, l:start_col] = lsp#utils#position#_lsp_to_vim('%', l:range['start'])
+    let [l:end_line, l:end_col] = lsp#utils#position#_lsp_to_vim('%', l:range['end'])
     if l:start_line ==# l:end_line
         let l:name = l:lines[l:start_line - 1][l:start_col - 1 : l:end_col - 2]
     else
diff --git a/autoload/lsp/ui/vim/diagnostics.vim b/autoload/lsp/ui/vim/diagnostics.vim
index ed534e45..8de662ea 100644
--- a/autoload/lsp/ui/vim/diagnostics.vim
+++ b/autoload/lsp/ui/vim/diagnostics.vim
@@ -68,10 +68,7 @@ function! lsp#ui#vim#diagnostics#get_diagnostics_under_cursor() abort
     let l:closest_distance = -1
 
     for l:diagnostic in l:diagnostics
-        let l:range = l:diagnostic['range']
-        let l:start_line = l:range['start']['line'] + 1
-        let l:start_char = l:range['start']['character']
-        let l:start_col = lsp#utils#to_col('%', l:start_line, l:start_char)
+        let [l:start_line, l:start_col] = lsp#utils#position#_lsp_to_vim('%', l:diagnostic['range']['start'])
 
         if l:line == l:start_line
             let l:distance = abs(l:start_col - l:col)
@@ -111,9 +108,7 @@ function! s:next_diagnostic(diagnostics) abort
     let l:next_line = 0
     let l:next_col = 0
     for l:diagnostic in a:diagnostics
-        let l:line = l:diagnostic['range']['start']['line'] + 1
-        let l:char = l:diagnostic['range']['start']['character']
-        let l:col = lsp#utils#to_col('%', l:line, l:char)
+        let [l:line, l:col] = lsp#utils#position#_lsp_to_vim('%', l:diagnostic['range']['start'])
         if l:line > l:view['lnum']
             \ || (l:line == l:view['lnum'] && l:col > l:view['col'] + 1)
             let l:next_line = l:line
@@ -124,9 +119,8 @@ function! s:next_diagnostic(diagnostics) abort
 
     if l:next_line == 0
         " Wrap to start
-        let l:next_line = a:diagnostics[0]['range']['start']['line'] + 1
-        let l:next_char = a:diagnostics[0]['range']['start']['character']
-        let l:next_col = lsp#utils#to_col('%', l:next_line, l:next_char) - 1
+        let [l:next_line, l:next_col] = lsp#utils#position#_lsp_to_vim('%', a:diagnostics[0]['range']['start'])
+        let l:next_col -= 1
     endif
 
     let l:view['lnum'] = l:next_line
@@ -172,9 +166,7 @@ function! s:previous_diagnostic(diagnostics) abort
     let l:next_col = 0
     let l:index = len(a:diagnostics) - 1
     while l:index >= 0
-        let l:line = a:diagnostics[l:index]['range']['start']['line'] + 1
-        let l:char = a:diagnostics[l:index]['range']['start']['character']
-        let l:col = lsp#utils#to_col('%', l:line, l:char)
+        let [l:line, l:col] = lsp#utils#position#_lsp_to_vim('%', a:diagnostics[l:index]['range']['start'])
         if l:line < l:view['lnum']
             \ || (l:line == l:view['lnum'] && l:col < l:view['col'])
             let l:next_line = l:line
@@ -186,9 +178,8 @@ function! s:previous_diagnostic(diagnostics) abort
 
     if l:next_line == 0
         " Wrap to end
-        let l:next_line = a:diagnostics[-1]['range']['start']['line'] + 1
-        let l:next_char = a:diagnostics[-1]['range']['start']['character']
-        let l:next_col = lsp#utils#to_col('%', l:next_line, l:next_char) - 1
+        let [l:next_line, l:next_col] = lsp#utils#position#_lsp_to_vim('%', a:diagnostics[-1]['range']['start'])
+        let l:next_col -= 1
     endif
 
     let l:view['lnum'] = l:next_line
diff --git a/autoload/lsp/ui/vim/diagnostics/textprop.vim b/autoload/lsp/ui/vim/diagnostics/textprop.vim
index 35905e32..91a1b28b 100644
--- a/autoload/lsp/ui/vim/diagnostics/textprop.vim
+++ b/autoload/lsp/ui/vim/diagnostics/textprop.vim
@@ -117,12 +117,8 @@ function! s:place_highlights(server_name, path, diagnostics) abort
     let l:bufnr = bufnr(a:path)
     if !empty(a:diagnostics) && l:bufnr >= 0
         for l:item in a:diagnostics
-            let l:start_line = l:item['range']['start']['line'] + 1
-            let l:start_char = l:item['range']['start']['character']
-            let l:start_col = lsp#utils#to_col(l:bufnr, l:start_line, l:start_char)
-            let l:end_line = l:item['range']['end']['line'] + 1
-            let l:end_char = l:item['range']['end']['character']
-            let l:end_col = lsp#utils#to_col(l:bufnr, l:end_line, l:end_char)
+            let [l:start_line, l:start_col] = lsp#utils#position#_lsp_to_vim(l:bufnr, l:item['range']['start'])
+            let [l:end_line, l:end_col] = lsp#utils#position#_lsp_to_vim(l:bufnr, l:item['range']['end'])
 
             let l:prop_type = s:get_prop_type(a:server_name, l:item['severity'])
             call prop_add(l:start_line, l:start_col, {
diff --git a/autoload/lsp/ui/vim/highlights.vim b/autoload/lsp/ui/vim/highlights.vim
index b6bfc282..b27d4496 100644
--- a/autoload/lsp/ui/vim/highlights.vim
+++ b/autoload/lsp/ui/vim/highlights.vim
@@ -95,11 +95,8 @@ function! s:place_highlights(server_name, path, diagnostics) abort
 
     if !empty(a:diagnostics) && l:bufnr >= 0
         for l:item in a:diagnostics
-            let l:line = l:item['range']['start']['line'] + 1
-            let l:start_char = l:item['range']['start']['character']
-            let l:start_col = lsp#utils#to_col(l:bufnr, l:line, l:start_char)
-            let l:end_char = l:item['range']['end']['character']
-            let l:end_col = lsp#utils#to_col(l:bufnr, l:line, l:end_char)
+            let [l:line, l:start_col] = lsp#utils#position#_lsp_to_vim(l:bufnr, l:item['range']['start'])
+            let [l:_, l:end_col] = lsp#utils#position#_lsp_to_vim(l:bufnr, l:item['range']['end'])
 
             let l:name = get(s:severity_sign_names_mapping, l:item['severity'], 'LspError')
             let l:hl_name = l:name . 'Highlight'
diff --git a/autoload/lsp/ui/vim/references.vim b/autoload/lsp/ui/vim/references.vim
index 73794f4a..8152ebb6 100644
--- a/autoload/lsp/ui/vim/references.vim
+++ b/autoload/lsp/ui/vim/references.vim
@@ -12,16 +12,10 @@ endif
 " positions, one for each line.
 " Return a list of positions.
 function! s:range_to_position(bufnr, range) abort
-    let l:start = a:range['start']
-    let l:end = a:range['end']
     let l:position = []
 
-    let l:start_line = l:start['line'] + 1
-    let l:start_char = l:start['character']
-    let l:start_col = lsp#utils#to_col(a:bufnr, l:start_line, l:start_char)
-    let l:end_line = l:end['line'] + 1
-    let l:end_char = l:end['character']
-    let l:end_col = lsp#utils#to_col(a:bufnr, l:end_line, l:end_char)
+    let [l:start_line, l:start_col] = lsp#utils#position#_lsp_to_vim(a:bufnr, a:range['start'])
+    let [l:end_line, l:end_col] = lsp#utils#position#_lsp_to_vim(a:bufnr, a:range['end'])
     if l:end_line == l:start_line
         let l:position = [[
         \ l:start_line,
diff --git a/autoload/lsp/ui/vim/utils.vim b/autoload/lsp/ui/vim/utils.vim
index 90d480d7..a7ca6545 100644
--- a/autoload/lsp/ui/vim/utils.vim
+++ b/autoload/lsp/ui/vim/utils.vim
@@ -25,9 +25,7 @@ function! lsp#ui#vim#utils#locations_to_loc_list(result) abort
     for l:location in l:locations
         if s:is_file_uri(l:location[l:uri])
             let l:path = lsp#utils#uri_to_path(l:location[l:uri])
-            let l:line = l:location[l:range]['start']['line'] + 1
-            let l:char = l:location[l:range]['start']['character']
-            let l:col = lsp#utils#to_col(l:path, l:line, l:char)
+            let [l:line, l:col] = lsp#utils#position#_lsp_to_vim(l:path, l:location[l:range]['start'])
 
             let l:index = l:line - 1
             if has_key(l:cache, l:path)
@@ -107,9 +105,7 @@ let s:diagnostic_severity = {
 
 function! s:symbols_to_loc_list_children(server, path, list, symbols, depth) abort
     for l:symbol in a:symbols
-        let l:line = l:symbol['range']['start']['line'] + 1
-        let l:char = l:symbol['range']['start']['character']
-        let l:col = lsp#utils#to_col(a:path, l:line, l:char)
+        let [l:line, l:col] = lsp#utils#position#_lsp_to_vim(a:path, l:symbol['range']['start'])
 
         call add(a:list, {
             \ 'filename': a:path,
@@ -138,9 +134,7 @@ function! lsp#ui#vim#utils#symbols_to_loc_list(server, result) abort
                 let l:location = l:symbol['location']
                 if s:is_file_uri(l:location['uri'])
                     let l:path = lsp#utils#uri_to_path(l:location['uri'])
-                    let l:line = l:location['range']['start']['line'] + 1
-                    let l:char = l:location['range']['start']['character']
-                    let l:col = lsp#utils#to_col(l:path, l:line, l:char)
+                    let [l:line, l:col] = lsp#utils#position#_lsp_to_vim(l:path, l:location['range']['start'])
                     call add(l:list, {
                         \ 'filename': l:path,
                         \ 'lnum': l:line,
@@ -152,9 +146,7 @@ function! lsp#ui#vim#utils#symbols_to_loc_list(server, result) abort
                 let l:location = a:result['request']['params']['textDocument']['uri']
                 if s:is_file_uri(l:location)
                     let l:path = lsp#utils#uri_to_path(l:location)
-                    let l:line = l:symbol['range']['start']['line'] + 1
-                    let l:char = l:symbol['range']['start']['character']
-                    let l:col = lsp#utils#to_col(l:path, l:line, l:char)
+                    let [l:line, l:col] = lsp#utils#position#_lsp_to_vim(l:path, l:symbol['range']['start'])
                     call add(l:list, {
                         \ 'filename': l:path,
                         \ 'lnum': l:line,
@@ -196,9 +188,7 @@ function! lsp#ui#vim#utils#diagnostics_to_loc_list(result) abort
                 let l:text .= l:item['code'] . ':'
             endif
             let l:text .= l:item['message']
-            let l:line = l:item['range']['start']['line'] + 1
-            let l:char = l:item['range']['start']['character']
-            let l:col = lsp#utils#to_col(l:path, l:line, l:char)
+            let [l:line, l:col] = lsp#utils#position#_lsp_to_vim(l:path, l:item['range'])
             call add(l:list, {
                 \ 'filename': l:path,
                 \ 'lnum': l:line,
diff --git a/autoload/lsp/utils.vim b/autoload/lsp/utils.vim
index 53670f3a..cd4310d0 100644
--- a/autoload/lsp/utils.vim
+++ b/autoload/lsp/utils.vim
@@ -186,23 +186,6 @@ function! lsp#utils#echo_with_truncation(msg) abort
     exec 'echo l:msg'
 endfunction
 
-" Convert a character-index (0-based) to byte-index (1-based)
-" This function requires a buffer specifier (expr, see :help bufname()),
-" a line number (lnum, 1-based), and a character-index (char, 0-based).
-function! lsp#utils#to_col(expr, lnum, char) abort
-    let l:lines = getbufline(a:expr, a:lnum)
-    if l:lines == []
-        if type(a:expr) != v:t_string || !filereadable(a:expr)
-            " invalid a:expr
-            return a:char + 1
-        endif
-        " a:expr is a file that is not yet loaded as a buffer
-        let l:lines = readfile(a:expr, '', a:lnum)
-    endif
-    let l:linestr = l:lines[-1]
-    return strlen(strcharpart(l:linestr, 0, a:char)) + 1
-endfunction
-
 " Convert a byte-index (1-based) to a character-index (0-based)
 " This function requires a buffer specifier (expr, see :help bufname()),
 " a line number (lnum, 1-based), and a byte-index (char, 1-based).
diff --git a/autoload/lsp/utils/position.vim b/autoload/lsp/utils/position.vim
index 77e7cb9b..24f0edef 100644
--- a/autoload/lsp/utils/position.vim
+++ b/autoload/lsp/utils/position.vim
@@ -1,4 +1,23 @@
-" @param bufnr = bufnr
+" This function can be error prone if the caller forgets to use +1 to vim line
+" so use lsp#utils#position#_lsp_to_vim instead
+" Convert a character-index (0-based) to byte-index (1-based)
+" This function requires a buffer specifier (expr, see :help bufname()),
+" a line number (lnum, 1-based), and a character-index (char, 0-based).
+function! s:to_col(expr, lnum, char) abort
+    let l:lines = getbufline(a:expr, a:lnum)
+    if l:lines == []
+        if type(a:expr) != v:t_string || !filereadable(a:expr)
+            " invalid a:expr
+            return a:char + 1
+        endif
+        " a:expr is a file that is not yet loaded as a buffer
+        let l:lines = readfile(a:expr, '', a:lnum)
+    endif
+    let l:linestr = l:lines[-1]
+    return strlen(strcharpart(l:linestr, 0, a:char)) + 1
+endfunction
+
+" @param expr = see :help bufname()
 " @param position = {
 "   'line': 1,
 "   'character': 1
@@ -10,6 +29,6 @@
 function! lsp#utils#position#_lsp_to_vim(expr, position) abort
     let l:line = a:position['line'] + 1
     let l:char = a:position['character']
-    let l:col = lsp#utils#to_col(a:expr, l:line, l:char)
+    let l:col = s:to_col(a:expr, l:line, l:char)
     return [l:line, l:col]
 endfunction
diff --git a/test/lsp/utils.vimspec b/test/lsp/utils.vimspec
index 2cdd4c6e..e32ab29e 100644
--- a/test/lsp/utils.vimspec
+++ b/test/lsp/utils.vimspec
@@ -82,34 +82,6 @@ Describe lsp#utils
         End
     End
 
-    Describe lsp#utils#to_col
-        It should return the byte-index from the given character-index on a buffer
-            call setline(1, ['a β c', 'δ', ''])
-            Assert lsp#utils#to_col('%', 1, 0) == 1
-            Assert lsp#utils#to_col('%', 1, 1) == 2
-            Assert lsp#utils#to_col('%', 1, 2) == 3
-            Assert lsp#utils#to_col('%', 1, 3) == 5
-            Assert lsp#utils#to_col('%', 1, 4) == 6
-            Assert lsp#utils#to_col('%', 1, 5) == 7
-            Assert lsp#utils#to_col('%', 2, 0) == 1
-            Assert lsp#utils#to_col('%', 2, 1) == 3
-            Assert lsp#utils#to_col('%', 3, 0) == 1
-            %delete
-        End
-
-        It should return the byte-index from the given character-index in an unloaded file
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 1, 0) == 1
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 1, 1) == 2
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 1, 2) == 3
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 1, 3) == 5
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 1, 4) == 6
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 1, 5) == 7
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 2, 0) == 1
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 2, 1) == 3
-            Assert lsp#utils#to_col('./test/testfiles/multibyte.txt', 3, 0) == 1
-        End
-    End
-
     Describe lsp#utils#to_char
         It should return the character-index from the given byte-index on a buffer
             call setline(1, ['a β c', 'δ', ''])