awesome print 2.0 #350

imajes · 2019-01-22T16:02:29Z

New approach for awesomeprint -- this uses a factory pattern to register formatters as they go, etc.

feedback welcome!

…ter formatters as they go. wip.

…n up more specs

…pple, nobrainer) and relo external formatters to formatters/ext

sigh. this needs to be done way better.

it's out of date, and I don't have time to migrate and validate it. Ideally, these 3rd party lib formatters should be added by that lib, rather than awesome_print introspecting into the lib.

imajes · 2019-01-23T20:42:47Z

@base10 this is the PR i'm talking about :D thx

baweaver · 2019-01-24T06:22:14Z

lib/awesome_print/colorize.rb

@@ -6,6 +6,8 @@ module Colorize
    # Pick the color and apply it to the given string as necessary.
    #------------------------------------------------------------------------------
    def colorize(str, type)
+      # puts "[COLORIZING] - using #{options[:color][type]} for #{type}" if AwesomePrint.debug
+


Forgot some commented code here?

baweaver · 2019-01-24T06:23:04Z

lib/awesome_print/formatter.rb

    end

-    # Main entry point to format an object.
+    # register a new formatter..


Nit: extra period

baweaver · 2019-01-24T06:23:57Z

lib/awesome_print/formatter.rb

+      # if that's not working, lets try discover the format via formattable?
+      self.class.registered_formatters.each do |fmt|
+        puts "[FIND] trying to use [#{fmt.first.to_s.greenish} - #{fmt.last.to_s.blue}] - core: #{fmt.last.core?}" if AwesomePrint.debug
+        #{fmt.last.core?}" if AwesomePrint.debug


commented code?

baweaver · 2019-01-24T06:24:55Z

lib/awesome_print/formatters/bigdecimal_formatter.rb

+
+module AwesomePrint
+  module Formatters
+    class BigdecimalFormatter < BaseFormatter


Curious, should this retain the capitalization of BigDecimal?

+1 on retaining BigDecimal capitalization.

baweaver · 2019-01-24T06:30:01Z

spec/formatters/array_spec.rb

+        ]
+    ]
+]
+EOS


If you use tilde heredocs you can preserve indentation which'll make this a bit easier to read and modify later:

http://www.virtuouscode.com/2016/01/06/about-the-ruby-squiggly-heredoc-syntax/

it 'plain multiline' do expect(@arr.ai(plain: true)).to eq <<~EOS.strip [ [0] 1, [1] :two, [2] "three", [3] [ [0] nil, [1] [ [0] true, [1] false ] ] ] EOS end

baweaver · 2019-01-24T06:32:51Z

lib/awesome_print/formatters/ext/sequel_model_class_formatter.rb

+      end
+
+      def format(object)
+        data = object.db_schema.inject({}) { |h, (prop, defn)| h.merge(prop => defn[:db_type]) }


With isolated state like inject or reduce it may make more sense to use merge! to avoid extra object allocations

baweaver · 2019-01-24T06:35:45Z

lib/awesome_print.rb

+require 'awesome_print/version'
+
+module AwesomePrint
+  class << self


You can also use extend self here instead of the singleton class trick.

baweaver · 2019-01-24T06:36:10Z

lib/awesome_print.rb

-      require 'awesome_print/ext/action_view'
+    # Class accessor to force colorized output (ex. forked subprocess where TERM
+    # might be dumb).
+    #---------------------------------------------------------------------------


Probably way overkill for this PR, but may make sense to look into YARDoc later as a standard documentation framework.

baweaver · 2019-01-24T06:38:06Z

lib/awesome_print/formatters/array_formatter.rb

+      def format(object)
+        @array = object
+
+        if object.length.zero?


Could use object.empty? here instead.

baweaver · 2019-01-24T06:44:58Z

lib/awesome_print/formatters/ext/active_model_error_formatter.rb

+            end
+            hash
+          end
+        end


Might consider breaking this out into a method, as it's getting a bit long. Perhaps something like this?

def format(object) if @options[:raw] return Formatters::ObjectFormatter.new(@inspector).format(object) end if !defined?(::ActiveSupport::OrderedHash) return Formatters::SimpleFormatter.new(@inspector).format(object.inspect) end object_dump = object.marshal_dump.first data = if object_dump.class.column_names != object_dump.attributes.keys object_dump.attributes else extract_column_values(object_dump) end data.merge!(details: object.details, messages: object.messages) "#{object} " << Formatters::HashFormatter.new(@inspector).format(data) end def extract_column_values(object_dump) object_dump .class .column_names .each_with_object(::ActiveSupport::OrderedHash.new) do |column_name, hash| if object_dump.has_attribute?(name) || object_dump.new_record? value = object_dump.respond_to?(name) ? object_dump.send(name) : object_dump.read_attribute(name) hash[name.to_sym] = value end end end

I like @baweaver's method extraction and think I'd go a step further of extracting:

data = if object_dump.class.column_names != object_dump.attributes.keys object_dump.attributes else extract_column_values(object_dump) end

to its own method, returning data.

baweaver · 2019-01-24T06:46:09Z

lib/awesome_print/formatters/ext/active_record_instance_formatter.rb

+              hash[name.to_sym] = value
+            end
+            hash
+          end


May be worth considering an ActiveRecordHelper type module full of some of these types of methods, as they're repeated in a few places.

baweaver · 2019-01-24T06:47:40Z

lib/awesome_print/formatters/ext/mongoid_document_formatter.rb

+      end
+
+      def format_as_instance(object)
+        data = (object.attributes || {}).sort_by { |key| key }.inject(::ActiveSupport::OrderedHash.new) do |hash, c|


Could use sort instead of sort_by here

baweaver · 2019-01-24T06:48:28Z

lib/awesome_print/formatters/ext/mongoid_document_formatter.rb

+        data = (object.attributes || {}).sort_by { |key| key }.inject(::ActiveSupport::OrderedHash.new) do |hash, c|
+          hash[c[0].to_sym] = c[1]
+          hash
+        end


ActiveSupport has symbolize_keys which you might find handy for this type of thing: https://apidock.com/rails/ActiveSupport/CoreExtensions/Hash/Keys/symbolize_keys

baweaver · 2019-01-24T06:54:17Z

lib/awesome_print/formatters/ext/nokogiri_document_formatter.rb

+        xml.gsub!(/>([^<]+)</) do |contents|
+          contents = colorize($1, :trueclass) if contents && !contents.empty?
+          ">#{contents}<"
+        end


May be worth considering a Nokogiri helper for some of these features.

Depending on how much you want to look into this code it may also make sense to transfer the regex into named and documented constants to make it clearer what this code does.

baweaver · 2019-01-24T06:57:33Z

spec/formatters/set_spec.rb

+      end
+    else # Prior to Ruby 1.9 the order of set values is unpredicatble.
+      it 'plain multiline' do
+        expect(@set.sort_by { |x| x.to_s }.ai(plain: true)).to eq(@arr.sort_by { |x| x.to_s }.ai(plain: true))


Could use sort_by(&:to_s) to save a few characters

baweaver · 2019-01-24T06:58:32Z

Let me know if you'd be interested in me going over anything in more detail, just a few suggestions and ideas for now.

base10 · 2019-01-25T20:57:46Z

lib/awesome_print/formatters/ext/active_record_attributeset_formatter.rb

+        # simple formatter for now
+        Formatters::ObjectFormatter.new(@inspector).format(object)
+      end
+


Extraneous spaces here?

base10 · 2019-01-25T20:58:52Z

lib/awesome_print/formatters/ext/active_model_error_formatter.rb

+
+        "#{object} " << Formatters::HashFormatter.new(@inspector).format(data)
+      end
+


Extraneous space between block ends here?

base10 · 2019-01-25T21:01:29Z

lib/awesome_print/formatters/ext/active_record_class_formatter.rb

+          "< #{colorize(object.superclass.to_s, :class)}",
+          Formatters::HashFormatter.new(@inspector).format(data)
+        ].join(' ')
+


Extraneous new line?

base10 · 2019-01-25T21:01:46Z

lib/awesome_print/formatters/ext/active_record_class_formatter.rb

+        ].join(' ')
+
+      end
+


Extraneous new line? (I see a lot of these below, too. If it's intentional 👍)

base10 · 2019-01-25T23:15:56Z

lib/awesome_print/formatters/hash_formatter.rb

      end

-      def format
+      # INSTANCE METHODS BELOW


Curious why the comment callout here and not with the other formatters.

gokul-infrrd · 2019-02-11T07:10:47Z

lib/awesome_print/formatters/base_formatter.rb

-      #------------------------------------------------------------------------------
-      def should_be_limited?
-        options[:limit] or (options[:limit].is_a?(Integer) and options[:limit] > 0)
+      def self.formattable?(obj)


Why this obj argument?

gokul-infrrd · 2019-02-11T07:11:59Z

lib/awesome_print/formatters/base_formatter.rb

      end

+      def format(object)


Is this argument required?

gokul-infrrd · 2019-02-11T08:36:55Z

lib/awesome_print/formatters/fallback_formatter.rb

+
+      formatter_for :self
+
+      def self.formattable?(object)


Why this object argument?

gokul-infrrd · 2019-02-11T08:39:50Z

lib/awesome_print/formatters/fallback_formatter.rb

+      #------------------------------------------------------------------------------
+      # FIXME: this could be super fixed.
+      #
+      def convert_to_hash(object)


Nice to have like this:-

Suggested change

def convert_to_hash(object)

def convert_to_hash(object)

def convert_to_hash(object) return nil if !object.respond_to?(:to_hash) || object.method(:to_hash).arity != 0 hash = object.to_hash return nil if !hash.respond_to?(:keys) || !hash.respond_to?('[]') hash end

imajes added 23 commits January 22, 2019 11:01

New approach for awesomeprint -- this uses a factory pattern to regis…

2105efb

…ter formatters as they go. wip.

splits spec, formalizes output for string/symbol

3738676

Further fix tests, set some formatters as core (too generic) and clea…

bb5e751

…n up more specs

remove extraneous formatters we don't want to continue supporting (ri…

7eaf2ca

…pple, nobrainer) and relo external formatters to formatters/ext

Remove extraneous/superflous copyright duplicates

102d254

Remove rails 2.x plugin style accessor

32e7efd

Remove the require 'ap' mechanic.

856f8c4

not going to support 1.x ruby anymore

e2bc9c2

Make external formatters (Rails, etc) format as expected)

f0ed2e2

Fix unbound methods

7f7c0bf

Fix more tests

bf1ff13

Fix nokogiri errors

1e5c6c9

fix load order...

751d6c9

Remove extra puts in spec.

e2acfab

Fix typo, and formally include date as a type here

a45b2ac

Remove no longer required active record stubs

5eb57ad

make debug reporting clearer, though this needs extracting

dbe414d

Include other obvious formatters

7ace70b

hack to support finding AR::B classes

da29f74

sigh. this needs to be done way better.

remove references to older rubies

52933a1

Fix mongoid concerns

1ff1206

Remove mongomapper support.

fe0786d

it's out of date, and I don't have time to migrate and validate it. Ideally, these 3rd party lib formatters should be added by that lib, rather than awesome_print introspecting into the lib.

Make this into a 2.0.0-pre gem

976e4a5

imajes requested review from maurogeorge and waldyr January 23, 2019 20:17

express the version correctly

87850a2

Fix loadpath issues that became apparent outside of the spec suite

8242bdf

baweaver reviewed Jan 24, 2019

View reviewed changes

lib/awesome_print/formatter.rb

end

# Main entry point to format an object.

# register a new formatter..

Copy link

baweaver Jan 24, 2019

Choose a reason for hiding this comment

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

Nit: extra period

baweaver reviewed Jan 24, 2019

View reviewed changes

base10 reviewed Jan 25, 2019

View reviewed changes

gokul-infrrd reviewed Feb 11, 2019

View reviewed changes

lib/awesome_print/formatters/base_formatter.rb

end

def format(object)

Copy link

gokul-infrrd Feb 11, 2019

Choose a reason for hiding this comment

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

Is this argument required?

gokul-infrrd reviewed Feb 11, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

awesome print 2.0 #350

awesome print 2.0 #350

imajes commented Jan 22, 2019

imajes commented Jan 23, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

base10 Jan 25, 2019

gokul-infrrd Feb 11, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

base10 Jan 25, 2019 •

edited

Loading

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver Jan 24, 2019

baweaver commented Jan 24, 2019

base10 Jan 25, 2019 •

edited

Loading

base10 Jan 25, 2019

base10 Jan 25, 2019

base10 Jan 25, 2019 •

edited

Loading

base10 Jan 25, 2019

gokul-infrrd Feb 11, 2019

gokul-infrrd Feb 11, 2019

gokul-infrrd Feb 11, 2019

gokul-infrrd Feb 11, 2019 •

edited

Loading


		"#{object} " << Formatters::HashFormatter.new(@inspector).format(data)
		end

awesome print 2.0 #350

Are you sure you want to change the base?

awesome print 2.0 #350

Conversation

imajes commented Jan 22, 2019

imajes commented Jan 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

base10 Jan 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baweaver commented Jan 24, 2019

base10 Jan 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

base10 Jan 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gokul-infrrd Feb 11, 2019 • edited Loading

Choose a reason for hiding this comment

base10 Jan 25, 2019 •

edited

Loading

base10 Jan 25, 2019 •

edited

Loading

base10 Jan 25, 2019 •

edited

Loading

gokul-infrrd Feb 11, 2019 •

edited

Loading