Skip to content

Added to_yaml & from_yaml (as well as to_hash & from_hash)#201

Closed
srushti wants to merge 1 commit into
mikel:masterfrom
srushti:c320ff436113017c0a7c1650990cda62b43b701a
Closed

Added to_yaml & from_yaml (as well as to_hash & from_hash)#201
srushti wants to merge 1 commit into
mikel:masterfrom
srushti:c320ff436113017c0a7c1650990cda62b43b701a

Conversation

@srushti
Copy link
Copy Markdown
Contributor

@srushti srushti commented Feb 9, 2011

Mail::Message#to_yaml used to give an error earlier.
TypeError: can't dump anonymous class Class

@mikel
Copy link
Copy Markdown
Owner

mikel commented Apr 16, 2011

Awesome, thanks for your work, however, the specs fail on the latest master. There was an error in the spec regexp which I fixed.

@mikel mikel closed this Apr 16, 2011
@mikel
Copy link
Copy Markdown
Owner

mikel commented Apr 16, 2011

I have merged this in.

@sandstrom
Copy link
Copy Markdown

Is this [pull request] compatible with YAML::dump? Unless I'm missing something it seems not.

>> m1 = Mail::Message.new
=> #<Mail::Message:2190471100, Multipart: false, Headers: >
>> YAML::dump m1
ArgumentError: wrong number of arguments (1 for 0)
    from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/yaml.rb:117:in `to_yaml'
    from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/yaml.rb:117:in `dump'
    from (irb):15
>> YAML::VERSION
=> "0.60"
>> Mail::VERSION.version
=> "2.2.19"
>> 

Also, the current serialization isn't a full representation of the Message object. Many instance variables aren't included, e.g. delivery_handler.

@sandstrom
Copy link
Copy Markdown

This is the implementation used in Object:

  YAML::quick_emit( self, opts ) do |out|
    out.map( taguri, to_yaml_style ) do |map|
      to_yaml_properties.each do |m|
        map.add( m[1..-1], instance_variable_get( m ) )
      end
    end
  end

urbanautomaton pushed a commit to urbanautomaton/mail that referenced this pull request May 17, 2011
urbanautomaton pushed a commit to urbanautomaton/mail that referenced this pull request May 17, 2011
@urbanautomaton
Copy link
Copy Markdown
Contributor

To address some of the issues raised by sandstrom (as I ran into precisely the same problem with the missing fields), I've made a couple of changes and submitted pull request #237 - details can be found there.

(I haven't addressed the YAML::dump issue though) Edit: now I have.

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.

5 participants