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

Issue #106: Fix I/O error handling in lexer and parser (attempt #3) #109

Merged
merged 11 commits into from
Nov 15, 2015

Conversation

RGafiyatullin
Copy link
Contributor

Issue #106: This time it should be okay.

The tests are passed.
The error is popped up to the very EventReader::next call.
The EventReader's source is accessible via er.source() and er.source_mut().

I'm still not sure whether it is convenient to dive into the cases of reader::Error though...

}
}

impl From<::util::CharReadError> for Error {
Copy link
Owner

Choose a reason for hiding this comment

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

It think it is better to add use util; than to add a global namespace qualifier everywhere. Or even use util::CharReadError.

@netvl
Copy link
Owner

netvl commented Nov 2, 2015

Looks really good to me, thank you very much! I have no other objections.

use super::ErrorKind::*;
match self.kind {
UnexpectedEof => &"Unexpected EOF",
Utf8( ref reason ) => error_description( reason ),
Copy link
Owner

Choose a reason for hiding this comment

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

This is a minor stylistic issue, but spaces inside parentheses is not idiomatic style in Rust programs. This applies for the enum declaration as well.

I should definitely check how rustfmt works, probably it makes sense to use it for xml-rs.

@netvl
Copy link
Owner

netvl commented Nov 15, 2015

Thank you very much! Sorry for the delay, I was really busy these two weeks :(

netvl added a commit that referenced this pull request Nov 15, 2015
Issue #106: Fix I/O error handling in lexer and parser (attempt #3)
@netvl netvl merged commit e273a02 into netvl:master Nov 15, 2015
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