Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new completion API #866
Add new completion API #866
Changes from all commits
2a13eb5
0812061
7cff503
5dd6977
0a15bf6
3ea2e42
0b58895
12e196e
8320d05
a9e7672
c43c278
e21b1a9
a86c249
2f5aeae
59c6c0b
534ae8a
c3d8807
40ef4be
ea29e0b
7d78abb
97ae903
ea4dd30
8a11721
f9d8bb9
793ec40
10ad58a
d2505b0
542b489
d5a5d62
d7fd65d
f46548a
ff9e0ae
2f504f7
896dcef
87ab9e3
1744b16
5782b46
316d29d
64627ee
8dd14b3
f47083e
542245e
44a3b2a
33496ac
7b154e6
c0805be
b528475
2497a45
9fe640a
a99cff7
49ccda6
cc475e9
61099bc
9e54c83
6fed5d3
dae5eba
1ccb7ab
036e432
d974c22
e3f580c
f207bf2
4468f8a
f38aaa0
bc687e5
07f7824
51bbd4d
b5c0de6
34ae9e1
18c47e3
6e596e4
9d0d7ea
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to correct me, I don't know if I used the new API correctly :)
Here is my feedback:
I want to highlight the differences between the new api, and the code before it:
The code before it did this:
row
,text
,cursor
position of each line, right before sending the request.END RESULT: It brought the lines in a state like they were when the completions were requested.
The code in this commit does this.
change_id
,change_region
(the region passed to thetransform_region_from
),text
andcursor
position for each line, right before sending the request.transform_region
.transform_region
is used for removing and inserting back the previous text for each line.END RESULT: It brings the lines in a state like they were when the completions were requested.
You may have noticed that I did this a bit differently, than what John did in his gist.
https://gist.github.com/jskinner/4dfa6d86f848e1e1e583883507369b67#file-async_edits-py-L238-L244
If I done the same thing as John did in his gist:
I would have to adjust the response from the server to account for all view edits that occurred,
and apply all TextChange-s to all completion items.
But that to me looks like a lot of work.
For me it looks easier to adjust the view state to the state it was when the request is send,
than to adjust the response from the server to account all the possible edits that may have had occurred.
Either way, I really want to hear the feedback from you all
And don't take me wrong with what I just said :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anything which prevents the
line
to be stored directly? Doing so would avoid convertingregion->tuple->region
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regions can't be serialized for transmission in
run_command
, which is required for the commit completion command.However, you should be able to use
tuple(region)
now that regions are iterable. I also think this step should happen at the communication edge were it is necessary to do so and not for the internal storage to make this kind of relation more obvious. I'm still not seing where the use case I assumed actually occurs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess the python 3.3 version of
sublime.Region
does not support iteration. This will be available as soon as we can move to python 3.8There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know you could do that
tuple(region)
with a Region. Cool :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Region' object is not iterable.
Looks like the LSP plugin is still using python 3.3.