Clients with `documentChanges` capability

[TinyLittleWheatley]

I'm getting an error for lsp_rename in some go file. The root of the problem is this line: https://github.com/johmsalas/text-case.nvim/blob/d62c63a4e9a996c7321885937ab89920fca2c1c8/lua/textcase/plugin/conversion.lua#L107

Which produces this error:

Error executing vim.schedule lua callback: vim/shared.lua:559: t: expected table, got nil

This is what my response looks like:

{
  result = {
    documentChanges = { {
        edits = { {
            newText = "AlphabetError",
            range = {
              ["end"] = {
                character = 16,
                line = 18
              },
              start = {
                character = 1,
                line = 18
              }
            }
          }, {
            newText = "AlphabetError",
            range = {
              ["end"] = {
                character = 42,
                line = 105
              },
              start = {
                character = 27,
                line = 105
              }
            }
          } },
        textDocument = {
          uri = "file:///F:/something/something/validations.go",
          version = 0
        }
      } }
  }
}

So i took a look at the specification and it seems like changes is an optional field of the interface and the edits might be on the documentChanges field based on the client capabilities. I propose:

          local files_count_by_current_response
          if response.result.changes ~= nil then
            files_count_by_current_response = vim.tbl_count(response.result.changes)
          else
            files_count_by_current_response = vim.tbl_count(response.result.documentChanges)
          end

Should i make a pr?

[Aphosis]

I also had this issue for a while (in Python buffers) and found this issue, the suggested patch worked great for me, thanks a lot !

[johmsalas]

Good catch!, @TinyLittleWheatley could you please create that PR? This is an example of how response.result.changes was mocked in the unit tests

[TinyLittleWheatley]

@johmsalas, I made the PR with mentioned changes. It doesn't change the unit tests though. I assume I should add a unit test that mocks the LSP behavior that caused the issue... right?

[johmsalas]

According to the documentation, documentChanges is the default value

If the client can handle versioned document edits and if documentChanges are present, the latter are preferred over changes

Something like

-- Comment explaining why it is important and defined in the specifications
describe("LSP changes and documentChanges", function()
  it("should use 'documentChanges' attribute when it is available")
    -- The mock would include both, taking into account changes is not really the one being used
    { result = { 
       changes = { ["file1"] = {}, ["file2"] = {},},
       documentChanges = { ["file3"] = {}, ["file4"] = {},},
    }}
    ...

  it("should use 'changes' attribute when 'documentChanges' is not available")
    -- The mock for this scenario would be like this:
    { result = { 
       changes = { ["file1"] = {}, ["file2"] = {},},
       documentChanges = nil,
    }}
    ...

The code above is just an example, please feel free to use the design you consider the best :)

To run the tests locally, the commands are in the justfile They would be run using just test in the root directory

[TinyLittleWheatley]

Thanks for the direction🙃. I've added the second unit test provided in your example but the first one seems kind of tricky to me. We don't really decide which field has to be applied... We choose one result object based on number of changes they make and pass it as is. Anyways the code is modified to count documentChanges instead of changes if both are available.

Also the documentation says(just before the sentence you quoted) that one of these fields(either ... or ...) should be set on result which confuses me.

[johmsalas]

This is merged :fireworks:. The added test is enough, I adjusted the commit message to adhere to Conventional Commits

Thank you so much for your contribution 😁