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

feat: Provide InlayHint for property expression in microprofile-config.properties #972

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

angelozerr
Copy link
Contributor

feat: Provide InlayHint for property expression in microprofile-config.properties (#971)

Fixes #971

@angelozerr angelozerr requested a review from fbricon June 23, 2023 15:58
@angelozerr angelozerr force-pushed the inlayhint-mp branch 2 times, most recently from 988a3c2 to b3fa3c5 Compare June 24, 2023 17:50
@angelozerr angelozerr marked this pull request as draft June 24, 2023 17:51
@angelozerr angelozerr force-pushed the inlayhint-mp branch 4 times, most recently from f997706 to 68968fe Compare June 25, 2023 08:01
@angelozerr
Copy link
Contributor Author

By default inlay hint for microprofile-config.properties are enabled, but of you don't want that, you can now disabled with UI settings:

image

You can now disable URL codelens if you wish:

image

Please note once you have changed th eenable/disable state you need to type something in the editor, because MicroProfile LS doesn't react to this settings changed (it should send an LSP refreshCodeLens and refreshInlayHint). I created the issues:

@angelozerr
Copy link
Contributor Author

I have improved InlayHint render to display it with hyperlink when there is a command (like I did for codelens):

IJInlayHintClickable

@fbricon
Copy link
Contributor

fbricon commented Jun 26, 2023

relates to #794

@adietish
Copy link
Contributor

@angelozerr: functionwise things work fine for me. I'm current reviewing the code. In LSPInlayHintInlayProvider methods are very long and variables crossreferenced. I thus tried to extract portions to their own methods to improve code readbility. Please have a look.

@angelozerr
Copy link
Contributor Author

@angelozerr: functionwise things work fine for me. I'm current reviewing the code. In LSPInlayHintInlayProvider methods are very long and variables crossreferenced. I thus tried to extract portions to their own methods to improve code readbility. Please have a look.

Indeed it looks better. Thanks! It should be nice if you could do the same thing for codelens (in an another PR).

Copy link
Contributor

@adietish adietish left a comment

Choose a reason for hiding this comment

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

LGTM. Added a commit where I tried to improve code readability.

private InlayPresentation toPresentation(Editor editor, int offset,
@NotNull
private static Range getViewPortRange(Editor editor) {
Position start = new Position(0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

seems wrong. The viewport's start range won't start from 0, if the editor was scrolled down.

Copy link
Contributor

Choose a reason for hiding this comment

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

which means you're likely unnecessarily computing hidden inlayhints

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that because the inlay hint is not refreshed when you are scrolling. Vscode provides this feature because when you scroll it refreshes the inlay hint. IJ doesn't support that it means that you need to consider that view port is the full document and not the visual view port.

Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment then, as it's not obvious

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I have add a comment

@sonarcloud
Copy link

sonarcloud bot commented Jun 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@angelozerr angelozerr merged commit 0828002 into redhat-developer:main Jun 27, 2023
@angelozerr
Copy link
Contributor Author

Tested on IC-2021.3 and EAP

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.

Provide InlayHint for property expression in microprofile-config.properties
3 participants