Skip to content

[MP Config 2.0] Support for config profiles#73

Merged
angelozerr merged 1 commit intoredhat-developer:masterfrom
angelozerr:profile
Sep 15, 2021
Merged

[MP Config 2.0] Support for config profiles#73
angelozerr merged 1 commit intoredhat-developer:masterfrom
angelozerr:profile

Conversation

@angelozerr
Copy link
Copy Markdown
Contributor

[MP Config 2.0] Support for config profiles

See eclipse-lsp4mp/lsp4mp#78

Signed-off-by: azerr azerr@redhat.com

See eclipse-lsp4mp/lsp4mp#78

Signed-off-by: azerr <azerr@redhat.com>
@angelozerr
Copy link
Copy Markdown
Contributor Author

To check this PR, create a file with profile like microprofile-config-dev.properties and open it, you should see at first that it's a MicroProfile properties:

image

an dyou should benefit with completion, etc MP support .

Copy link
Copy Markdown
Contributor

@AlexXuChen AlexXuChen left a comment

Choose a reason for hiding this comment

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

If I create a file, named microprofile-config-.properties, I will get the Properties language mode and not MicroProfile properties. Is that intentional, or should we not allow microprofile-config- as a properties file?

Comment thread package.json
"microprofile-config.properties"
"filenamePatterns": [
"microprofile-config.properties",
"microprofile-config-?*.properties"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just curious. Does the new pattern also cover the old one ? Not that I mind repeating if the purpose is to make it very clear.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you remove microprofile-config.properties from the patterns, then it will no longer recognize microprofile-config.properties as a "Microprofile properties" file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

microprofile-config.properties should work, no?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it works if you have "microprofile-config.properties" in "filenamePatterns", but I think Roland was wondering if just "microprofile-config-?*.properties" alone will support "microprofile-config.properties", which it does not.

Copy link
Copy Markdown
Member

@rgrunber rgrunber Sep 14, 2021

Choose a reason for hiding this comment

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

Ok I see. So ? matches a single character, and not (0 or 1) of the previous character.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For package.json, I believe it matches exactly one character, but yes, it usually means the character it follows is optional. If the latter were the case, then microprofile-config-.properties should be recognized as a MicroProfile properties file, but it isn't.

@angelozerr
Copy link
Copy Markdown
Contributor Author

If I create a file, named microprofile-config-.properties, I will get the Properties language mode and not MicroProfile properties. Is that intentional, or should we not allow microprofile-config- as a properties file?

yes it's intentional, microprofile-config-.properties is not a valid profile.

@AlexXuChen
Copy link
Copy Markdown
Contributor

microprofile-config--.properties however is recognized as a valid MicroProfile properties file

@angelozerr
Copy link
Copy Markdown
Contributor Author

It means that profile is just - . I dont know if there is some validation for profile name. I suggest that you try to start an applicationwith the profile - and see if it retrievers the proper property on the microprofile-config--.properties

The pattern that I write is just to be sure that there is a profile

@angelozerr angelozerr added this to the 0.4.0 milestone Sep 15, 2021
@angelozerr angelozerr merged commit e09b876 into redhat-developer:master Sep 15, 2021
@angelozerr
Copy link
Copy Markdown
Contributor Author

Thanks for your reviews @AlexXuChen @rgrunber

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