Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

editsToUse are deeper in LSP WorkspaceEdit response #2647

Merged
merged 1 commit into from
Oct 24, 2018
Merged

editsToUse are deeper in LSP WorkspaceEdit response #2647

merged 1 commit into from
Oct 24, 2018

Conversation

Siprj
Copy link
Contributor

@Siprj Siprj commented Oct 23, 2018

Edits are actually stored in changes field in WorkspaceEdit; see:
https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#workspaceedit

Resolves: #2646

@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #2647 into master will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2647      +/-   ##
=========================================
+ Coverage   45.79%   45.8%   +<.01%     
=========================================
  Files         361     361              
  Lines       14625   14624       -1     
  Branches     1924    1924              
=========================================
  Hits         6698    6698              
+ Misses       7697    7696       -1     
  Partials      230     230
Impacted Files Coverage Δ
browser/src/Services/Workspace/Workspace.ts 23.4% <0%> (+0.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7431882...afbbe58. Read the comment docs.

@akinsho
Copy link
Member

akinsho commented Oct 24, 2018

@Siprj thanks for looking into this and raising a PR, I noticed this a part of #1717 but got bogged down with other changes, I wonder re. the change if you could swap the variable to work as follows:

 const editsToUse = edits.documentChanges
            ? convertTextDocumentChangesToFileMap(edits.documentChanges)
            : edits.changes

It's a small change but the re-assignment using let is a little more verbose and this was the change I was gonna (eventually) propose in #1717 (which would help me out as it would reduce conflicts there

@Siprj
Copy link
Contributor Author

Siprj commented Oct 24, 2018

@Akin909 Done.

Copy link
Member

@akinsho akinsho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Siprj thanks for the contribution and for updating the PR 👍 💯

@akinsho akinsho merged commit c4dd8ad into onivim:master Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants