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

Changed option :raw to true as default. #191

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mattmartini
Copy link

In response to Issue #190 AP upgrade loses functionality in displaying objects.
https://github.com/michaeldv/awesome_print/issues/190

Expected behavior is that Objects are recursively formatted.
The default setting is reversed to true.

Expected behavior is that Objects are recursively formatted.
The default setting is reversed to true.
@maurogeorge
Copy link
Collaborator

@mattmartini thanks for the PR. I will review soon.

@@ -38,7 +38,7 @@ Default options:
:html => false, # Use ANSI color codes rather than HTML.
:multiline => true, # Display in multiple lines.
:plain => false, # Use colors.
:raw => false, # Do not recursively format object instance variables.
:raw => true, # Recursively format object instance variables.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think we need to change defaults, since this is a breaking change and brokes the semantic versioning.

@maurogeorge
Copy link
Collaborator

Do you think it is possible that we have formatted objects like

#<Item:0x7ffbb19c12d8
    @count = 0,
    attr_accessor :quantity = 3,
    attr_reader :name = "box",
    attr_reader :value = 2.5
>

without touch the the raw option and the AR, Mongoid and Ripple. What I understand about nested objects is like something:

Raw as false

#<Item:0x7ffbb19c12d8
    @count = 0,
    attr_accessor :quantity = 3,
    attr_reader :name = "box",
    attr_reader :value = 2.5
    attr_reader :child = #<Item:0x007ffc1c93aad9 @name="box", @quantity=3, @value=2.5, @count=0>
>

Raw as true

#<Item:0x7ffbb19c12d8
    @count = 0,
    attr_accessor :quantity = 3,
    attr_reader :name = "box",
    attr_reader :value = 2.5
    attr_reader :child = #<Item:0x007ffc1c93aad9
                                       @name="box",
                                       @quantity=3,
                                       @value=2.5
                                       @count=0
                                  >
>

I think this is the expected behavior. What you think?

Thanks for the PR 👍

@mattmartini
Copy link
Author

Mauro,
Please let me know how you would like me to proceed. I thought that we had said that the expected behavior should be that object instance variables and accessors are shown by default. Since the current behavior is the opposite with the :raw defaulting to false, changing the default to true would reverse this and thus achieve the intended behavior.
I'm open to your ideas.
Matt

OK, It seems we were writing at the same time. I just saw your comment. I will look into it.

@mattmartini
Copy link
Author

So what you would like is that objects behave as if :raw was true for the first level, but not to recurse.
I agree with this, and will attempt the changes. Would we need a new switch to turn this behavior off? Seems like without this behavior we might as well tun AP off, so I don't think so.

@maurogeorge
Copy link
Collaborator

@mattmartini I guess that the idea, agree with you 😄
Thanks ❤️

@gerrywastaken
Copy link
Contributor

Just to help me keep track of this PR and for anybody else who taking a look: @waldyr added a comment on the associated issue asking about the status of the PR: #190 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants