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

Added indent with indicators to yaml-v12 #406

Merged
merged 12 commits into from
Jun 14, 2024

Conversation

aheiska
Copy link
Contributor

@aheiska aheiska commented Nov 9, 2023

Enables to indent yaml like:

root:
  - item1
  - item2

This applies only to circe-yaml-v12. I could not come up with a way to do it for circe-yaml in a binary compatible way (I would assume this is binary compatible).

Test copied from #398 and implemented PrinterBuilder builder as suggested there.

I also added deprecations. Values are just a guess and maintainers can of course change then as they see fit.

@aheiska aheiska changed the title Added indent with indicators to yaml- Added indent with indicators to yaml-v12 Nov 9, 2023
@aheiska
Copy link
Contributor Author

aheiska commented Nov 13, 2023

Also made relevant changes to circe-yaml. That required implementation of PrinterBuilder & PrinterImpl to circe-yaml. They are mostly independent of old Printer (notable exception being Printer.Yamlversion). I would assume this allows extending the printer configuration api in a backwards compatible way. As always, maintainer are welcome to add/edit depreciations in a way they see fit.

Also the io.circe.yaml.PrinterBuilder has SnakeStringStyle extension in it (could not come up with a better place). We cannot use one from common, because snake-yaml versions use different ScalarStyle. Other circe-yaml spesific setting enums are pattern matched at PrinterBuilder.build but .toScalarStyle is used also from PrinterImpl

Copy link
Contributor

@hamnis hamnis left a comment

Choose a reason for hiding this comment

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

LGTM, but @zarthross will need to have the final word

@hamnis hamnis requested a review from zarthross June 13, 2024 13:09
Copy link
Member

@zarthross zarthross left a comment

Choose a reason for hiding this comment

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

Few changes need to be made, but otherwise it looked good.

@aheiska
Copy link
Contributor Author

aheiska commented Jun 14, 2024

Thank you for your review. I made the changes you requested.

@hamnis hamnis requested a review from zarthross June 14, 2024 13:11
Copy link
Member

@zarthross zarthross left a comment

Choose a reason for hiding this comment

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

Great work! Thank your for the contribution!

@zarthross zarthross merged commit b28a5b1 into circe:main Jun 14, 2024
6 checks passed
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

4 participants