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

Serialization of Properties from String #1173

Closed

Conversation

daniel-jasinski
Copy link
Contributor

Second part of implementation for 1152.

@sandwwraith
Copy link
Member

Just a quick heads up, I'll go through this PR in details next week: your tests seem to fail for Kotlin/JS target. Also, test functions should be named in camel case, not snake_case.

@sandwwraith sandwwraith self-requested a review November 6, 2020 11:49
@daniel-jasinski
Copy link
Contributor Author

Any feedback on this PR?

The issue with test were already fixed.

@sandwwraith
Copy link
Member

Yes, sorry for the delay.
Regarding the PR: after some design discussions, we've came to the idea that kotlinx.serialization's Properties should have the same API and behavior as Java's properties, without any additional settings, such as separators. This allows us keep API simple and predictable, and reduces number of public classes to maintain — inevitably, there will be PR with 'add this separator and that separator' because they do not conform to any traditional standard.

I understand that additional separators fit to your particular use-case, but I am not sure if this kind of properties file is popular enough to be useful to lot of people. However, I can suggest you the following:

  1. Align PR with Java Properties API (without separator/unicode setting, line separator should be taken from the host OS). Also, name encodeToString kinda misleading and clashes with JSON, so we probably can use something like encodeToMulitlineString.
  2. You can take current implementation (it's Apache 2 anyway) and make your own format on it, Rich Properties or something, with settings for separators etc. — we can accept it to formats list as a third-party format.
    Hope this explanation was helpful.

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