Repository: autozimu/LanguageClient-neovim (language: rust)

Issue:
Cursor jumps when clangd automatically inserts headers
- Did you upgrade to latest plugin version?
Yes
- Did you upgrade to/compile latest binary? Run shell command
  `bin/languageclient --version` to get its version number.

I compiled the latest master

- (Neovim users only) Did you check output of `:checkhealth LanguageClient`?
Everything healthy
- Did you check [troubleshooting]?
Yes

[troubleshooting]: https://github.com/autozimu/LanguageClient-neovim/blob/next/INSTALL.md#6-troubleshooting

## Describe the bug
Clangd has the following CLI option:
```
  --header-insertion=<value>      - Add #include directives when accepting code completions
    =iwyu                         -   Include what you use. Insert the owning header for top-level symbols, unless the header is alread
```
When it automatically includes a header an additional line gets inserted at the top of the file. So my cursor should move one line down in document but it stays at the same position.
Because of that I start editing the previous line in the document.

## Environment
- neovim/vim version (`nvim --version` or `vim --version`):
```
NVIM v0.5.0-144-g568900806
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
Compilation: /usr/bin/gcc-5 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -O2 -g -DMIN_LOG_LEVEL=3 -Og -g -Wall -Wextra -pedantic -Wno-unused-p
arameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wmissing-prototypes -Wvla -fstack-protector-strong -fdiagnostics-color=a
uto -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -I/home/travis/build/neovim/bot
-ci/build/neovim/build/config -I/home/travis/build/neovim/bot-ci/build/neovim/src -I/home/travis/build/neovim/bot-ci/build/neovim/.deps
/usr/include -I/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/build/src/nvim/auto -I/home/travis/build/neovim/bot-ci/buil
d/neovim/build/include
Compiled by travis@travis-job-3c2d1eac-da9f-48a2-905f-74efb0c9d5e5

Features: +acl +iconv +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/share/nvim"

```
- This plugin version (`git rev-parse --short HEAD`):
dd45e31
- This plugin's binary version (`bin/languageclient --version`):
built from dd45e31
- language server: clangd-10 (https://apt.llvm.org/)
- Minimal vimrc content (A minimal vimrc is the smallest vimrc that could
  reproduce the issue. Refer to an example [here][min-vimrc.vim]):
```
call plug#begin('~/.local/share/nvim/plugged2')

Plug 'autozimu/LanguageClient-neovim', {
    \ 'branch': 'next',
    \ 'do': 'bash install.sh',
    \ }
if has('nvim')
  Plug 'Shougo/deoplete.nvim', { 'do': ':UpdateRemotePlugins' }
else
  Plug 'Shougo/deoplete.nvim'
  Plug 'roxma/nvim-yarp'
  Plug 'roxma/vim-hug-neovim-rpc'
endif

call plug#end()


augroup filetype_rust
      autocmd!
      autocmd BufReadPost *.cpp setlocal filetype=cpp
augroup END

autocmd FileType * call deoplete#custom#option('auto_complete', v:true)


" Always draw sign column. Prevent buffer moving when adding/deleting sign.
 set signcolumn=yes

let g:deoplete#enable_at_startup = 1
 let g:LanguageClient_serverCommands = {
     \ 'cpp': ['clangd-10', '--clang-tidy', '--header-insertion=iwyu', '--background-index', '--suggest-missing-includes']
         \ }
 let $RUST_BACKTRACE = 1
 let g:LanguageClient_loggingLevel = 'INFO'
 let g:LanguageClient_virtualTextPrefix = ''
 let g:LanguageClient_loggingFile = expand('~/.local/share/nvim/LanguageClient.log')
 let g:LanguageClient_serverStderr = expand('~/.local/share/nvim/LanguageServer.log')

```
[min-vimrc.vim]: https://github.com/autozimu/LanguageClient-neovim/blob/next/min-vimrc.vim

## To Reproduce

Create the following file and start the language server:
```
  #include <string>                                                
  auto main() -> int                                    
  {                                                                                                                               
          auto foo = std::make
  }                                                                                                               
```
Then complete for instance to `std::make_signed`. After entering the opening paren iwyu (inlude what you use) automatically inserts the indirectly used header but the cursor will jump one line.

```
  #include <string>                       
  #include <type_traits>                                
  auto main() -> int                                    
  {<<< cursor position will be here                                                                                                                               
✖         auto foo = std::make_signed( Use of class template 'make_signed' requires template arguments                                                               
✖ } Expected expression                                                                                          
                           
```


## Current behavior
Cursor jumps one line

## Expected behavior
Cursor position should be changed when the language server changed the document.

## Screenshots
N/A

## Additional context
Don't know whether this bug is specific to the clangd version.



Discussion / hints:
Yes, I can confirm this behavior. In fact, the cursor does not jump but when the header file is inserted, the cursor is not adjusted properly. This is very annoying. Correct me if I am wrong. 

Fix patch:
diff --git a/src/language_server_protocol.rs b/src/language_server_protocol.rs
index 4eef63d8..8ec9c8f1 100644
--- a/src/language_server_protocol.rs
+++ b/src/language_server_protocol.rs
@@ -279,14 +279,22 @@ impl LanguageClient {
             match changes {
                 DocumentChanges::Edits(ref changes) => {
                     for e in changes {
-                        self.apply_TextEdits(&e.text_document.uri.filepath()?, &e.edits)?;
+                        position = self.apply_TextEdits(
+                            &e.text_document.uri.filepath()?,
+                            &e.edits,
+                            position,
+                        )?;
                     }
                 }
                 DocumentChanges::Operations(ref ops) => {
                     for op in ops {
                         match op {
                             Edit(ref e) => {
-                                self.apply_TextEdits(&e.text_document.uri.filepath()?, &e.edits)?
+                                position = self.apply_TextEdits(
+                                    &e.text_document.uri.filepath()?,
+                                    &e.edits,
+                                    position,
+                                )?
                             }
                             Op(ref rop) => match rop {
                                 Create(file) => {
@@ -302,7 +310,7 @@ impl LanguageClient {
             }
         } else if let Some(ref changes) = edit.changes {
             for (uri, edits) in changes {
-                self.apply_TextEdits(&uri.filepath()?, edits)?;
+                position = self.apply_TextEdits(&uri.filepath()?, edits, position)?;
             }
         }
         self.edit(&None, &filename)?;
@@ -433,10 +441,15 @@ impl LanguageClient {
         Ok(())
     }
 
-    fn apply_TextEdits<P: AsRef<Path>>(&self, path: P, edits: &[TextEdit]) -> Fallible<()> {
+    fn apply_TextEdits<P: AsRef<Path>>(
+        &self,
+        path: P,
+        edits: &[TextEdit],
+        position: Position,
+    ) -> Fallible<Position> {
         debug!("Begin apply TextEdits: {:?}", edits);
         if edits.is_empty() {
-            return Ok(());
+            return Ok(position);
         }
 
         let mut edits = edits.to_vec();
@@ -459,7 +472,7 @@ impl LanguageClient {
             lines.push("".to_owned());
         }
 
-        let mut lines = apply_TextEdits(&lines, &edits)?;
+        let (mut lines, position) = apply_TextEdits(&lines, &edits, &position)?;
 
         if lines.last().map(String::is_empty) == Some(true) && fixendofline {
             lines.pop();
@@ -470,7 +483,7 @@ impl LanguageClient {
         }
         self.vim()?.rpcclient.notify("setline", json!([1, lines]))?;
         debug!("End apply TextEdits");
-        Ok(())
+        Ok(position)
     }
 
     fn update_quickfixlist(&self) -> Fallible<()> {
@@ -2739,7 +2752,7 @@ impl LanguageClient {
             return Ok(());
         }
 
-        self.apply_TextEdits(filename, &edits)?;
+        let position = self.apply_TextEdits(filename, &edits, position)?;
         self.vim()?
             .cursor(position.line + 1, position.character + 1)
     }
diff --git a/src/utils.rs b/src/utils.rs
index 0fd3a7fc..5f168d08 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -130,7 +130,89 @@ impl<P: AsRef<Path> + std::fmt::Debug> ToUrl for P {
     }
 }
 
-pub fn apply_TextEdits(lines: &[String], edits: &[TextEdit]) -> Fallible<Vec<String>> {
+fn position_to_offset(lines: &[String], position: &Position) -> usize {
+    if lines.is_empty() {
+        return 0;
+    }
+
+    let line = std::cmp::min(position.line as usize, lines.len() - 1);
+    let character = std::cmp::min(position.character as usize, lines[line].len());
+
+    let chars_above: usize = lines[..line].iter().map(|text| text.len() + 1).sum();
+    chars_above + character
+}
+
+#[test]
+fn test_position_to_offset() {
+    assert_eq!(position_to_offset(&[], &Position::new(0, 0)), 0);
+
+    let lines: Vec<String> = "\n".lines().map(ToOwned::to_owned).collect();
+    assert_eq!(position_to_offset(&lines, &Position::new(0, 0)), 0);
+    assert_eq!(position_to_offset(&lines, &Position::new(0, 1)), 0);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 0)), 0);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 1)), 0);
+
+    let lines: Vec<String> = "a\n".lines().map(ToOwned::to_owned).collect();
+    assert_eq!(position_to_offset(&lines, &Position::new(0, 0)), 0);
+    assert_eq!(position_to_offset(&lines, &Position::new(0, 1)), 1);
+    assert_eq!(position_to_offset(&lines, &Position::new(0, 2)), 1);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 0)), 0);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 1)), 1);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 2)), 1);
+
+    let lines: Vec<String> = "a\nbc\n".lines().map(ToOwned::to_owned).collect();
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 0)), 2);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 1)), 3);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 2)), 4);
+    assert_eq!(position_to_offset(&lines, &Position::new(1, 3)), 4);
+}
+
+fn offset_to_position(lines: &[String], offset: usize) -> Position {
+    if lines.is_empty() {
+        return Position::new(0, 0);
+    }
+
+    let mut offset = offset;
+    for (line, text) in lines.iter().enumerate() {
+        if offset <= text.len() {
+            return Position::new(line as u64, offset as u64);
+        }
+
+        offset -= text.len() + 1;
+    }
+
+    let last_line = lines.len() - 1;
+    let last_character = lines[last_line].len();
+    Position::new(last_line as u64, last_character as u64)
+}
+
+#[test]
+fn test_offset_to_position() {
+    assert_eq!(offset_to_position(&[], 0), Position::new(0, 0));
+
+    let lines: Vec<String> = "\n".lines().map(ToOwned::to_owned).collect();
+    assert_eq!(offset_to_position(&lines, 0), Position::new(0, 0));
+    assert_eq!(offset_to_position(&lines, 1), Position::new(0, 0));
+
+    let lines: Vec<String> = "a\n".lines().map(ToOwned::to_owned).collect();
+    assert_eq!(offset_to_position(&lines, 0), Position::new(0, 0));
+    assert_eq!(offset_to_position(&lines, 1), Position::new(0, 1));
+    assert_eq!(offset_to_position(&lines, 2), Position::new(0, 1));
+
+    let lines: Vec<String> = "a\nbc\n".lines().map(ToOwned::to_owned).collect();
+    assert_eq!(offset_to_position(&lines, 0), Position::new(0, 0));
+    assert_eq!(offset_to_position(&lines, 1), Position::new(0, 1));
+    assert_eq!(offset_to_position(&lines, 2), Position::new(1, 0));
+    assert_eq!(offset_to_position(&lines, 3), Position::new(1, 1));
+    assert_eq!(offset_to_position(&lines, 4), Position::new(1, 2));
+    assert_eq!(offset_to_position(&lines, 5), Position::new(1, 2));
+}
+
+pub fn apply_TextEdits(
+    lines: &[String],
+    edits: &[TextEdit],
+    position: &Position,
+) -> Fallible<(Vec<String>, Position)> {
     // Edits are ordered from bottom to top, from right to left.
     let mut edits_by_index = vec![];
     for edit in edits {
@@ -153,13 +235,32 @@ pub fn apply_TextEdits(lines: &[String], edits: &[TextEdit]) -> Fallible<Vec<Str
     }
 
     let mut text = lines.join("\n");
+    let mut offset = position_to_offset(&lines, &position);
     for (start, end, new_text) in edits_by_index {
         let start = std::cmp::min(start, text.len());
         let end = std::cmp::min(end, text.len());
         text = String::new() + &text[..start] + new_text + &text[end..];
+
+        // Update offset only if the edit's entire range is before it.
+        // Edits after the offset do not affect it.
+        // Edits covering the offset cause unpredictable effect.
+        if end <= offset {
+            offset += new_text.len();
+            offset -= new_text.matches("\r\n").count(); // line ending is counted as one offset
+            offset -= std::cmp::min(offset, end - start);
+        }
     }
 
-    Ok(text.lines().map(ToOwned::to_owned).collect())
+    offset = std::cmp::min(offset, text.len());
+
+    let new_lines: Vec<String> = text.lines().map(ToOwned::to_owned).collect();
+    let new_position = offset_to_position(&new_lines, offset);
+    debug!(
+        "Position change after applying text edits: {:?} -> {:?}",
+        position, new_position
+    );
+
+    Ok((new_lines, new_position))
 }
 
 #[test]
@@ -198,7 +299,12 @@ fn test_apply_TextEdit() {
         .to_owned(),
     };
 
-    assert_eq!(apply_TextEdits(&lines, &[edit]).unwrap(), expect);
+    let position = Position::new(0, 0);
+
+    // Ignore returned position since the edit's range covers current position and the new position
+    // is undefined in this case
+    let (result, _) = apply_TextEdits(&lines, &[edit], &position).unwrap();
+    assert_eq!(result, expect);
 }
 
 #[test]
@@ -221,7 +327,86 @@ fn test_apply_TextEdit_overlong_end() {
         new_text: r#"nb = 123"#.to_owned(),
     };
 
-    assert_eq!(apply_TextEdits(&lines, &[edit]).unwrap(), expect);
+    let position = Position::new(0, 1);
+
+    let (result, _) = apply_TextEdits(&lines, &[edit], &position).unwrap();
+    assert_eq!(result, expect);
+}
+
+#[test]
+fn test_apply_TextEdit_position() {
+    let lines: Vec<String> = "abc = 123".lines().map(|l| l.to_owned()).collect();
+
+    let expected_lines: Vec<String> = "newline\nabcde = 123"
+        .lines()
+        .map(|l| l.to_owned())
+        .collect();
+
+    let edits = [
+        TextEdit {
+            range: Range {
+                start: Position {
+                    line: 0,
+                    character: 1,
+                },
+                end: Position {
+                    line: 0,
+                    character: 3,
+                },
+            },
+            new_text: "bcde".to_owned(),
+        },
+        TextEdit {
+            range: Range {
+                start: Position {
+                    line: 0,
+                    character: 0,
+                },
+                end: Position {
+                    line: 0,
+                    character: 0,
+                },
+            },
+            new_text: "newline\n".to_owned(),
+        },
+    ];
+
+    let position = Position::new(0, 4);
+    let expected_position = Position::new(1, 6);
+
+    assert_eq!(
+        apply_TextEdits(&lines, &edits, &position).unwrap(),
+        (expected_lines, expected_position)
+    );
+}
+
+#[test]
+fn test_apply_TextEdit_CRLF() {
+    let lines: Vec<String> = "abc = 123".lines().map(|l| l.to_owned()).collect();
+
+    let expected_lines: Vec<String> = "a\r\nbc = 123".lines().map(|l| l.to_owned()).collect();
+
+    let edit = TextEdit {
+        range: Range {
+            start: Position {
+                line: 0,
+                character: 0,
+            },
+            end: Position {
+                line: 0,
+                character: 1,
+            },
+        },
+        new_text: "a\r\n".to_owned(),
+    };
+
+    let position = Position::new(0, 2);
+    let expected_position = Position::new(1, 1);
+
+    assert_eq!(
+        apply_TextEdits(&lines, &[edit], &position).unwrap(),
+        (expected_lines, expected_position)
+    );
 }
 
 pub trait Combine {


Test patch: