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: Resolve system properties/environment variables while browsing the application.properties values #456

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

angelozerr
Copy link
Contributor

@angelozerr angelozerr commented Aug 14, 2024

feat: Resolve system properties/environment variables while browsing the application.properties values

Fixes #448

Here the result:

image

//cc @gastaldi

@angelozerr angelozerr mentioned this pull request Aug 14, 2024
@angelozerr angelozerr force-pushed the sysenv_resolve branch 4 times, most recently from 8a5a549 to 19697a8 Compare August 14, 2024 14:12
@datho7561
Copy link
Contributor

datho7561 commented Aug 14, 2024

Hovering on the system property/environment variable names causes an error popup, and the following is logged in the language server logs:

Request failed: Request microprofile/propertyDocumentation failed with message: Command 'microprofile/propertyDocumentation' must be called with required MicroProfilePropertyDocumentationParams.sourceType! (-32603).

@angelozerr
Copy link
Contributor Author

@datho7561 I cannot reproduce it, could you share a simple example please.

@datho7561
Copy link
Contributor

datho7561 commented Aug 14, 2024

Opening completions at the |:

quarkus.application.name=${ASDF|}

I am given options corresponding to environment variables. Accepting the suggestion ASDF_DIR results in

quarkus.application.name=|

update: this only seems to be happening to the environment variable results, the Java property ones work fine.

@datho7561
Copy link
Contributor

I cannot reproduce it, could you share a simple example please.

hovering-environment-variable-fails.mp4

The value of ASDF_DIR is /home/davthomp/.asdf

@angelozerr angelozerr force-pushed the sysenv_resolve branch 2 times, most recently from cbd2fbb to d0d87ac Compare August 14, 2024 17:19
@angelozerr
Copy link
Contributor Author

Hovering on the system property/environment variable names causes an error popup, and the following is logged in the language server logs:

Hovering on the system property/environment variable names causes an error popup, and the following is logged in the language server logs:

Request failed: Request microprofile/propertyDocumentation failed with message: Command 'microprofile/propertyDocumentation' must be called with required MicroProfilePropertyDocumentationParams.sourceType! (-32603).

It should be fixed.

@angelozerr
Copy link
Contributor Author

update: this only seems to be happening to the environment variable results, the Java property ones work fine.

They were several issues with completion default items that I have fixed, it should work correctly now. I need to write tests again.

@angelozerr angelozerr force-pushed the sysenv_resolve branch 2 times, most recently from 28f3d75 to e07a229 Compare August 15, 2024 09:18
@datho7561
Copy link
Contributor

Sounds good. I checked out the latest commit and confirmed that's working. Once the tests are in place I think it's ready to go.

@angelozerr angelozerr force-pushed the sysenv_resolve branch 10 times, most recently from 9ecce12 to 8011bdb Compare August 16, 2024 13:59
@angelozerr angelozerr force-pushed the sysenv_resolve branch 2 times, most recently from 6fa4c50 to da276c5 Compare August 16, 2024 14:27
application.properties values

Fixes eclipse#448

Signed-off-by: azerr <azerr@redhat.com>
@angelozerr angelozerr marked this pull request as ready for review August 16, 2024 14:44
@angelozerr angelozerr merged commit aed533b into eclipse:master Aug 16, 2024
2 checks passed
@angelozerr
Copy link
Contributor Author

Sounds good. I checked out the latest commit and confirmed that's working. Once the tests are in place I think it's ready to go.

Thanks @datho7561 ! I wrote a lot of tests and I merge it since CI build is working well.

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.

Resolve system properties/environment variables while browsing the application.properties values
4 participants