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.

Sign up for GitHub

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

Jump to bottom

feat: add a config option to exclude declaration from LSP references #6886

Merged
merged 3 commits into from
Apr 27, 2023
Merged

feat: add a config option to exclude declaration from LSP references #6886

merged 3 commits into from
Apr 27, 2023

Conversation

mcdoker18
Copy link
Contributor

Fixes: #5344

pascalkuthe
pascalkuthe previously approved these changes Apr 26, 2023
View reviewed changes
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

LGTM, I erpsonally prefer turning this off so it's nice to have a config option for this 👍

@pascalkuthe pascalkuthe added this to the 23.04 milestone Apr 26, 2023
pascalkuthe
pascalkuthe previously approved these changes Apr 26, 2023
View reviewed changes
@mcdoker18
Copy link
Contributor Author

Personally, I'd like to have this option disabled by default. Because we have the go to declaration action. I have never wanted to find all usage and declarations of a function at the same time.

@pascalkuthe
Copy link
Member

Personally, I'd like to have this option disabled by default. Because we have the go to declaration action. I have never wanted to find all usage and declarations of a function at the same time.

There was some discussion about this in #5344 I personally also prefer the option to be disabled but I think @the-mikedavis and @archseer prefer the current default. I think it might be a difference of people who come from vscode/nvim vs intellij. At the end of the day this is personal preference and I think both are fine defaults. It's pretty easy to change so I don't think it's that important what the default is

the-mikedavis
the-mikedavis requested changes Apr 26, 2023
View reviewed changes
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Ah actually one more minor thing

helix-term/src/commands/lsp.rs Outdated Show resolved Hide resolved
@mcdoker18 mcdoker18 force-pushed the feat/declaration_goto_references branch from 54f1e8c to d0240eb Compare April 27, 2023 07:46
@the-mikedavis the-mikedavis changed the title feat: added the config option to exclude declaration from reference query feat: add a config option to exclude declaration from LSP references Apr 27, 2023
@pascalkuthe pascalkuthe merged commit 2836ea2 into helix-editor:master Apr 27, 2023
@effinsky
Copy link

thanks, all!

Triton171 pushed a commit to Triton171/helix that referenced this pull request Jun 18, 2023
@mcdoker18 @Triton171
feat: add a config option to exclude declaration from LSP references (h…
90c056b 
…elix-editor#6886)

* feat: added the config option to exclude declaration from reference query

Fixes: helix-editor#5344

* fix: review

* fix: review
wes-adams pushed a commit to wes-adams/helix that referenced this pull request Jul 4, 2023
@mcdoker18 @wes-adams
feat: add a config option to exclude declaration from LSP references (h…
be3ccee 
…elix-editor#6886)

* feat: added the config option to exclude declaration from reference query

Fixes: helix-editor#5344

* fix: review

* fix: review
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
@mcdoker18
feat: add a config option to exclude declaration from LSP references (h…
35bfd32 
…elix-editor#6886)

* feat: added the config option to exclude declaration from reference query

Fixes: helix-editor#5344

* fix: review

* fix: review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pascalkuthe pascalkuthe pascalkuthe approved these changes

@the-mikedavis the-mikedavis the-mikedavis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
23.05
Development

Successfully merging this pull request may close these issues.

Add option to exclude the definition from references in LSP searches
4 participants
@mcdoker18 @pascalkuthe @effinsky @the-mikedavis