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

Deviate from ruby/pretty_print and enrich the API #57

Merged
merged 23 commits into from
Jan 10, 2025
Merged

Conversation

stackmystack
Copy link
Collaborator

Addresses many points in #55

@stackmystack
Copy link
Collaborator Author

  1. I caught a couple of issues with separate.
  2. Surround is still not implemented
  3. We should not forget to add ruby 3.0+ to the CI.

examples/oppen_and_wadler_customization/space.rb Outdated Show resolved Hide resolved
Comment on lines +212 to +213
case delim
in nil then ['', '']
in String | Symbol then [delim, delim]
in Array then delim.values_at(0, 1).map(&:to_s)
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should extract this part to mixins since we have meen using it twice.

lib/wadler/print.rb Outdated Show resolved Hide resolved
@@ -9,13 +9,13 @@
# By using a callable:
space = ->(n) { '---' * n }

printer = Oppen::Wadler.new(space: space)
printer = Oppen::Wadler.new(indent: 2, space: space)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

Comment on lines +8 to +9
printer_width_default = Oppen::Wadler.new(indent: 2)
printer_width_narrow = Oppen::Wadler.new(indent: 2, width: width)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

Comment on lines 7 to 9
# @return [Integer]
# the starting indentation level for the whole printer.
attr_reader :base_indent
Copy link
Collaborator

Choose a reason for hiding this comment

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

You never set the value of @base_indent. Maybe you intended a getter for @indent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's wrong indeed. base indent is current indent. Maybe we should also kill the setter. It doesn't make sense.

Comment on lines +39 to +37
# @param indent [Integer]
# the default indentation amount for {group} and {nest}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Despite being longer, I would much rather go with default_indent. We can live with indent if you find it too long.

Makes for a simpler, more declarative API.
It's still there since project initialization.
@stackmystack stackmystack force-pushed the rich-api branch 2 times, most recently from 4a10fd0 to 39e72cc Compare January 7, 2025 08:27
@Am1ne77 Am1ne77 force-pushed the rich-api branch 2 times, most recently from 7fa5121 to 9fbc705 Compare January 7, 2025 12:35
@stackmystack stackmystack merged commit d5a8c04 into main Jan 10, 2025
2 checks passed
@stackmystack stackmystack deleted the rich-api branch January 10, 2025 09:22
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.

2 participants