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

Disable comma separation in values in ZIO for complex configs handled by zio-config #1114

Merged
merged 21 commits into from
Mar 9, 2023

Conversation

afsalthaj
Copy link
Collaborator

@afsalthaj afsalthaj commented Mar 9, 2023

ZIO-Config currently depends fully on ZIO library for its indexed config formats too.
However, as a consequence we reintroduced certain bugs related to the fact that values were considered as a list if there exists a comma. This can hurt a lot of users, who would normally consider list as something that hinted by specific characters such a starting [ and close ] in Yaml or - in YML . This implies, for example, users expects only ["a", "b", "c"] to be a list and not "a, b, c". If they need "a,b,c" to be considered as a list, then they can set enableCommaSeparatedValueAsList = true. The name is long to understand what's going on.

With ZIO-3.0, we can avoid this slight weirdness

@afsalthaj afsalthaj requested a review from a team as a code owner March 9, 2023 19:46
@afsalthaj afsalthaj merged commit 1c9d98d into series/4.x Mar 9, 2023
@@ -59,7 +58,8 @@ object TypesafeConfigProvider {

ConfigProvider.fromMap(
indexedMapWithHiddenDelimiter,
pathDelim = hiddenDelim
pathDelim = hiddenDelim,
seqDelim = if(enableCommaSeparatedValueAsList) "," else hiddenDelim
Copy link
Contributor

@guersam guersam Mar 10, 2023

Choose a reason for hiding this comment

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

Would it be better if the user can provide their own separator like | ? For example, with valueSeparator: Option[String] this line can be like this:

seqDelim = valueSeparator.getOrElse(hiddenDelim)

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.

None yet

2 participants