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

Implement codeLens #209

Draft
wants to merge 4 commits into
base: staging
Choose a base branch
from

Conversation

WilsonZiweiWang
Copy link
Contributor

Added a simple codeLens for function references and a setting to control this behaviour.
image

Ticket: 14622

@WilsonZiweiWang WilsonZiweiWang self-assigned this Apr 30, 2024
@WilsonZiweiWang WilsonZiweiWang changed the base branch from main to staging April 30, 2024 21:12
idillon-sfl
idillon-sfl previously approved these changes May 3, 2024
},
"bitbake.enableCodeLensReferencesOnFunctions": {
"type": "boolean",
"default": false,
Copy link
Member

Choose a reason for hiding this comment

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

I'm not very familiar with CodeLens. So you're disabling it by default on purpose, right?

Copy link
Member

Choose a reason for hiding this comment

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

this surprised me also. However since currently we only have code lens for references, which have their dedicated button, I'm not sure the feature adds much value right now. Meanwhile it would bloat the interface.

Copy link
Member

Choose a reason for hiding this comment

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

This demo is cool however I think codelenses are justified when we have specific commands to run. There are few extensions doing so and the additional buttons could annoy users. A useful example for instance is the git extension which adds additional commands for comparing with other branches.

Copy link
Member

Choose a reason for hiding this comment

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

I think we would need different commands in order for this to be useful. I cannot think of one that isn't already seemlessly integrated into the interface yet (value hovers, rename, definitions, references...).

@@ -148,6 +153,37 @@ disposables.push(
analyzer.clearRecipeLocalFiles()
}),

connection.onCodeLens(async (params): Promise<LSP.CodeLens[]> => {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you'd like to put this into a onCodeLens file in connectionHandlers, but this works too

Copy link
Member

Choose a reason for hiding this comment

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

Yes, like we do other feature handlers, it would be nice to have a dedicated file for separating features.

idillon-sfl
idillon-sfl previously approved these changes May 15, 2024
},
"bitbake.enableCodeLensReferencesOnFunctions": {
"type": "boolean",
"default": false,
Copy link
Member

Choose a reason for hiding this comment

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

this surprised me also. However since currently we only have code lens for references, which have their dedicated button, I'm not sure the feature adds much value right now. Meanwhile it would bloat the interface.

},
"bitbake.enableCodeLensReferencesOnFunctions": {
"type": "boolean",
"default": false,
Copy link
Member

Choose a reason for hiding this comment

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

This demo is cool however I think codelenses are justified when we have specific commands to run. There are few extensions doing so and the additional buttons could annoy users. A useful example for instance is the git extension which adds additional commands for comparing with other branches.

@@ -148,6 +153,37 @@ disposables.push(
analyzer.clearRecipeLocalFiles()
}),

connection.onCodeLens(async (params): Promise<LSP.CodeLens[]> => {
Copy link
Member

Choose a reason for hiding this comment

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

Yes, like we do other feature handlers, it would be nice to have a dedicated file for separating features.

},
"bitbake.enableCodeLensReferencesOnFunctions": {
"type": "boolean",
"default": false,
Copy link
Member

Choose a reason for hiding this comment

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

I think we would need different commands in order for this to be useful. I cannot think of one that isn't already seemlessly integrated into the interface yet (value hovers, rename, definitions, references...).

@idillon-sfl
Copy link
Member

I moved onCodeLens to its own file and rebased so we don't have to wait for Ziwei to merge

@WilsonZiweiWang WilsonZiweiWang marked this pull request as draft July 11, 2024 13:41
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.

3 participants