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

Allow to ignore invalid fields #28

Merged
merged 3 commits into from
Nov 22, 2016
Merged

Allow to ignore invalid fields #28

merged 3 commits into from
Nov 22, 2016

Conversation

solenko
Copy link
Contributor

@solenko solenko commented Nov 21, 2016

In some cases you do not want to loose all data if you have only one invalid field in encoded VCard (especially for multi-contact vcards).

This change add two config parameters
Vcard.configuration.raise_on_invalid_line (default true).
When this parameter is set to true, decode will raise Vcard::InvalidEncodingError if found a field that encoded incorrectly (default behavior).
When raise_on_invalid_line = false, exception will not be raised but field will be marked as invalid.

With parameter Vcard.configuration.ignore_invalid_vcards you can define behavior for invalid fields.
If ignore_invalid_vcards = true, then Vcard.decode will ignore a vcard that have at least one incorrect field.
With ignore_invalid_vcards = false, vcard only invalid fields will be ignored, but vcard will be added to list, returned by .decode

Anton Petrunich and others added 2 commits November 21, 2016 12:11
In some cases you do not want to loose all data if you have only one invalid field in encoded VCard (especially for multi-contact vcards).

This change add two config parameters
::Vcard.configuration.raise_on_invalid_line (default true).
When this parameter is set to true, decode will raise ::Vcard::InvalidEncodingError if found a field that encoded incorrectly (default behavior).
When raise_on_invalid_line = false, exception will not be raised but field will be marked as invalid.

With parameter ::Vcard.configuration.ignore_invalid_vcards you can define behavior for invalid fields.
If ignore_invalid_vcards = true, then ::Vcard.decode will ignore a vcard that have at least one incorrect field.
With ignore_invalid_vcards = false, vcard only invalid fields will be ignored, but vcard will be added to list, returned by .decode
Copy link
Collaborator

@brendon brendon left a comment

Choose a reason for hiding this comment

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

Thanks @solenko! We really appreciate your contribution. I've edited your README changes to fix the english and left a couple of comments for you to consider. Can you action those and them I'm happy to merge this.

@fields = []

fields.each do |f|
raise ArgumentError, "fields must be an array of DirectoryInfo::Field objects" unless f.kind_of? DirectoryInfo::Field
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there be a condition on this line regarding whether or not to raise?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't change behavior in this place. It looks for me like a strict type check and it's not clear for me why it needed here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries :)

@ignore_invalid_vcard = true
end

private
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove this given there are no private methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@brendon brendon merged commit e9450c5 into qoobaa:master Nov 22, 2016
@brendon
Copy link
Collaborator

brendon commented Nov 22, 2016

Thanks again Anton :) This is all merged now. @qoobaa, I can release a new gem version if you like. You just need to grant me access to Rubygems.

@qoobaa
Copy link
Owner

qoobaa commented Nov 22, 2016

@brendon done

@brendon
Copy link
Collaborator

brendon commented Nov 22, 2016

@solenko, this has been released as 0.2.14.

@brendon
Copy link
Collaborator

brendon commented Nov 22, 2016

Thanks @qoobaa :)

@solenko
Copy link
Contributor Author

solenko commented Nov 23, 2016

@brendon thank you for fast reaction and especially for fixes of my English :)

@brendon
Copy link
Collaborator

brendon commented Nov 23, 2016

You're welcome :)

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.

3 participants