Skip to content
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

Support for LSP/tsserver Code Actions #3437

Merged
merged 26 commits into from
Nov 14, 2020
Merged

Support for LSP/tsserver Code Actions #3437

merged 26 commits into from
Nov 14, 2020

Conversation

daliusd
Copy link
Contributor

@daliusd daliusd commented Nov 14, 2020

Related issue #1466

There might be situations when the error is reported by multiple sources
at the same location (e.g. eslint and tsserver). In that case first
source might be missing `code`.
This is done to accommodate the fact that in the future there will be
more capabilities: refactorings from tsserver and LSP Code Actions.
We will lose one testing layer but I think it is acceptable in this
situation.
It seems edits assumes that lines includes end-of-line symbol.
Meanwhile Vim's readfile command omits new lines (\n) and carriage
return (\r) symbols if there are any.
Previously code was applied from top-to-bottom that required extra
parameters to track progress and there was bug. I have changed code to
bottom-to-top approach as that does not require those extra parameters
and solved the bug.
E.g.: jedi-language-server returns null in some cases.
Tested with typescript-language-server and it is working.
Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

This is great. Let's just do it.

I'll modify the things I've mentioned here.

@@ -33,35 +33,35 @@ endfunction

function! s:ChangeCmp(left, right) abort
if a:left.start.line < a:right.start.line
return -1
return 1
endif
Copy link
Member

Choose a reason for hiding this comment

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

It makes more sense for the cmp function to sort forwards. Apply reverse() to reverse the List below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.


Code Actions support for `tsserver`.

There are two different kind of actions supported for `tsserver`. If run in
Copy link
Member

Choose a reason for hiding this comment

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

There are some typos here to fix.

If there are multiple code fixes available use will be shown input to choose
one. In visual mode `tsserver` will be queries for applicable refactors
(e.g. extract to constant or extract to function) and user will be given
choice to select the one he/she likes.
Copy link
Member

Choose a reason for hiding this comment

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

I tend to just write "they" as a gender neutral term.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. That sounds better.

@@ -617,6 +617,7 @@ function! ale#completion#ParseLSPCompletions(response) abort
let l:user_data = {'_ale_completion_item': 1}

if has_key(l:item, 'additionalTextEdits')
\ && l:item.additionalTextEdits isnot v:null
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
\ && l:item.additionalTextEdits isnot v:null
\&& l:item.additionalTextEdits isnot v:null

Preference is to not add a space.

let l:end_column = min([l:end_column, len(getline(l:end_line))])

let l:Callback = function(
\ 's:OnReady', [l:line, l:column, l:end_line, l:end_column])
Copy link
Member

Choose a reason for hiding this comment

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

I tend to put the closing parenthesis on its own line.

@w0rp w0rp merged commit 01800a2 into dense-analysis:master Nov 14, 2020
@w0rp
Copy link
Member

w0rp commented Nov 14, 2020

Cheers! 🍻

@daliusd
Copy link
Contributor Author

daliusd commented Nov 14, 2020

Cheers! 🍻

@daliusd daliusd deleted the codeactions branch November 14, 2020 11:50
ivorpeles pushed a commit to ivorpeles/ale that referenced this pull request Nov 22, 2020
* Added tsserver and LSP code action support.
* tsserver refactors support added.
* Handling special case when new text is added after new line symbol.
* ale#code_action#ApplyChanges simplified.
* Initial attempt on LSP Code Actions.
* workspace/executeCommand added.
* Some null checks added.
* Add last column to LSP Code Action message.
* Pass diagnostics to LSP code action.

Previously ApplyChanges code was applied from top-to-bottom that required 
extra parameters to track progress and there was bug. I have changed code
to bottom-to-top approach as that does not require those extra parameters
and solved the bug.

Tested with typescript-language-server and it is working.
benknoble added a commit to benknoble/ale that referenced this pull request Nov 30, 2020
* origin/master: (40 commits)
  fix: correct suggested filetype for yamlfix
  feat: add yamlfix fixer
  Use _config for LSP config options
  Add support for R languageserver (dense-analysis#3370)
  Fix 3103 - add shellcheck shell directive detection. (dense-analysis#3216)
  Added the Vundle command in installation instructions (dense-analysis#3400)
  Adds support for Tlint - A Tighten Opinionated PHP Linter (dense-analysis#3291)
  Add php phpcbf options (dense-analysis#3383)
  Use has('gui_running') instead of has('gui')
  Close dense-analysis#2727 - Add a hover-only setting for balloons
  Fix dense-analysis#3332 - Modify everything for rename/actions
  Add a missing blank line in documentation
  Add luafmt fixer (dense-analysis#3289)
  dense-analysis#3442 Fix code fix clangd issue
  Close dense-analysis#1466 - Add GVIM refactor menu support
  Look for node packages in .yarn/sdks as well
  Update documentation for code actions and rename
  cmp forwards, and reverse the code actions
  Support for LSP/tsserver Code Actions (dense-analysis#3437)
  Move the test for buffer-local variables
  ...
jsit added a commit to jsit/ale that referenced this pull request Dec 26, 2020
* origin/master: (164 commits)
  fix: correct suggested filetype for yamlfix
  feat: add yamlfix fixer
  Use _config for LSP config options
  Add support for R languageserver (dense-analysis#3370)
  Fix 3103 - add shellcheck shell directive detection. (dense-analysis#3216)
  Added the Vundle command in installation instructions (dense-analysis#3400)
  Adds support for Tlint - A Tighten Opinionated PHP Linter (dense-analysis#3291)
  Add php phpcbf options (dense-analysis#3383)
  Use has('gui_running') instead of has('gui')
  Close dense-analysis#2727 - Add a hover-only setting for balloons
  Fix dense-analysis#3332 - Modify everything for rename/actions
  Add a missing blank line in documentation
  Add luafmt fixer (dense-analysis#3289)
  dense-analysis#3442 Fix code fix clangd issue
  Close dense-analysis#1466 - Add GVIM refactor menu support
  Look for node packages in .yarn/sdks as well
  Update documentation for code actions and rename
  cmp forwards, and reverse the code actions
  Support for LSP/tsserver Code Actions (dense-analysis#3437)
  Move the test for buffer-local variables
  ...
motato1 pushed a commit to motato1/ale that referenced this pull request Jan 11, 2021
* Added tsserver and LSP code action support.
* tsserver refactors support added.
* Handling special case when new text is added after new line symbol.
* ale#code_action#ApplyChanges simplified.
* Initial attempt on LSP Code Actions.
* workspace/executeCommand added.
* Some null checks added.
* Add last column to LSP Code Action message.
* Pass diagnostics to LSP code action.

Previously ApplyChanges code was applied from top-to-bottom that required 
extra parameters to track progress and there was bug. I have changed code
to bottom-to-top approach as that does not require those extra parameters
and solved the bug.

Tested with typescript-language-server and it is working.
jsit added a commit to jsit/ale that referenced this pull request Apr 19, 2021
* master: (35 commits)
  fix: correct suggested filetype for yamlfix
  feat: add yamlfix fixer
  Use _config for LSP config options
  Add support for R languageserver (dense-analysis#3370)
  Fix 3103 - add shellcheck shell directive detection. (dense-analysis#3216)
  Added the Vundle command in installation instructions (dense-analysis#3400)
  Adds support for Tlint - A Tighten Opinionated PHP Linter (dense-analysis#3291)
  Add php phpcbf options (dense-analysis#3383)
  Use has('gui_running') instead of has('gui')
  Close dense-analysis#2727 - Add a hover-only setting for balloons
  Fix dense-analysis#3332 - Modify everything for rename/actions
  Add a missing blank line in documentation
  Add luafmt fixer (dense-analysis#3289)
  dense-analysis#3442 Fix code fix clangd issue
  Close dense-analysis#1466 - Add GVIM refactor menu support
  Look for node packages in .yarn/sdks as well
  Update documentation for code actions and rename
  cmp forwards, and reverse the code actions
  Support for LSP/tsserver Code Actions (dense-analysis#3437)
  feat: add autoimport fixer
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants