Multiple Definitions Returned #449

thchha · 2022-08-12T19:14:01Z

Hey Mr. Bosch,

thank you for your efforts.
I am using your lsp-client implementation as a reference for my own language server since it is the most complaint one. Your code looks great. :)

This pull request requests your opinion on the prompted change.
I would appreciate mentoring in adapting the pull request to your needs, if the essence is acceptable.

Sincerly,
Thomas

quickfixlist with the options to select. Selecting an option should be in terms of the replicated vim behaviour (my assumption). This _could_ be useful, if the langauge is lexical scoped. There should be only _one declaration_. Multiple _re_definitions could appear.

…ons. This branch is concerned with compability. It should: * enable jumping to (1) declaration => with split, only one binding * extend the existing documentation => reference parent impl * extend the existing tests => copy dart-test

thchha · 2022-08-12T20:46:12Z

I created a secondary branch, which introduces the old behaviour, but with the request of textDocument/declaration. You can find (and merge/refer to it without obligations) to it at https://github.com/thchha/vim-lsc/tree/declaration .

The user behaviour changes for the following:

When one binding is provided, both bindings (C-W or just C-]) is working as expected.
When there are multiple bindings, both bindings will call copen().

The complementary branch will enable the previous behavior on a differing binding called gd, which is mnemonic and free (and requests textDocument/declaration).

note that textDocument/declaration can return multiple bindings as well; But I agree that this should not be the case.

If there is only a single item in an array, diretly chose this. If the result isn't an array, we can use it similiarly. Somehow, I incremented this check. This fixes it.

…ons. This branch is concerned with compability. It should: * enable jumping to (1) declaration => with split, only one binding * extend the existing documentation => reference parent impl * extend the existing tests => copy dart-test

thchha · 2022-12-07T00:46:25Z

@natebosch You can test the behaviour of my PR in unity at the following branch: https://github.com/thchha/vim-lsc/tree/thchha

Please be aware that I am new to contributing. Just pinpoint the issues you have in your gut.
I will dedicate these and adapt, until it proves the quality to be squashed.

…ons. This branch is concerned with compability. It should: * enable jumping to (1) declaration => with split, only one binding * extend the existing documentation => reference parent impl * extend the existing tests => copy dart-test

(Multiple Definitions Returned natebosch#449) natebosch#449 71181e0

natebosch · 2023-03-25T21:02:49Z

autoload/lsc/config.vim

@@ -3,6 +3,8 @@ if !exists('s:initialized')
  let s:default_maps = {
      \ 'GoToDefinition': '<C-]>',
      \ 'GoToDefinitionSplit': ['<C-W>]', '<C-W><C-]>'],
+      \ 'GoToDeclaration': 'gd',
+      \ 'GoToDeclarationSplit': ['<C-W>]', '<C-W>gd'],


I don't think we can map '<C-W>]' again since it's mapped for GoToDefinitionSplit

Suggested change

\ 'GoToDeclarationSplit': ['<C-W>]', '<C-W>gd'],

\ 'GoToDeclarationSplit': ['<C-W>]', '<C-W>gd'],

natebosch · 2023-03-25T21:05:26Z

doc/lsc.txt

@@ -75,6 +76,15 @@ will default to opening a vertical split, while >
 <
 will prefer a new tab.

+                                                *:LSClientGoToDeclaration*
+Similiar to |LSClientGoToDefinition| but expects only a single result when


I don't think we should treat declarations specially since they allow multiple too.

What do you think about always jumping to the first element in the list for either case, but also populating the quickfix list with the extras when there are more?

…ons. This branch is concerned with compability. It should: * enable jumping to (1) declaration => with split, only one binding * extend the existing documentation => reference parent impl * extend the existing tests => copy dart-test

(Multiple Definitions Returned natebosch#449) natebosch#449 71181e0

…definitions." This reverts commit 46d3564.

Thomas added 4 commits August 12, 2022 21:02

necessary documentation updates.

11c7bf2

do not execute, but call the function to open the quickfix list instead.

67e4d1a

Thomas Hage and others added 2 commits August 12, 2022 23:03

documentation should refer to the default binding.

5c26752

If the length of the result contains multiple items, prompt copen.

1ee6df6

If there is only a single item in an array, diretly chose this. If the result isn't an array, we can use it similiarly. Somehow, I incremented this check. This fixes it.

kohnish added a commit to kohnish/vim-lsc that referenced this pull request Mar 3, 2023

copen was broken

d641260

(Multiple Definitions Returned natebosch#449) natebosch#449 71181e0

natebosch reviewed Mar 25, 2023

View reviewed changes

kohnish added a commit to kohnish/vim-lsc that referenced this pull request Mar 27, 2023

copen was broken

a48fb8a

(Multiple Definitions Returned natebosch#449) natebosch#449 71181e0

kohnish added a commit to kohnish/vim-lsc that referenced this pull request Jun 30, 2023

Revert "the PR natebosch#449 requests the ability to handle multiple …

2a1b853

…definitions." This reverts commit 46d3564.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple Definitions Returned #449

Multiple Definitions Returned #449

thchha commented Aug 12, 2022

thchha commented Aug 12, 2022 •

edited

thchha commented Dec 7, 2022

natebosch Mar 25, 2023

natebosch Mar 25, 2023

	\ 'GoToDeclarationSplit': ['<C-W>]', '<C-W>gd'],
	\ 'GoToDeclarationSplit': ['<C-W>]', '<C-W>gd'],

Multiple Definitions Returned #449

Are you sure you want to change the base?

Multiple Definitions Returned #449

Conversation

thchha commented Aug 12, 2022

thchha commented Aug 12, 2022 • edited

thchha commented Dec 7, 2022

natebosch Mar 25, 2023

Choose a reason for hiding this comment

natebosch Mar 25, 2023

Choose a reason for hiding this comment

thchha commented Aug 12, 2022 •

edited