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

Ignore UTF-8 BOM from the xml input #54

Merged
merged 2 commits into from May 3, 2018

Conversation

ryochack
Copy link
Contributor

@ryochack ryochack commented May 2, 2018

This PR is to support the SVD input that contains UTF-8 BOM. (Ignore BOM)
Please refer to the svd2rust's PR.

Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

A small nitpick but otherwise it looks good

src/lib.rs Outdated
#[test]
fn test_trim_utf8_bom_from_str() {
// UTF-8 BOM + "xyz"
let bom_str = str::from_utf8(&[0xefu8, 0xbbu8, 0xbfu8, 0x78u8, 0x79u8, 0x7au8]).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Same as earlier comment, could you use b"\xef\xbb\xbfxyz"

src/lib.rs Outdated
@@ -128,9 +128,26 @@ impl ElementExt for Element {
}
}

/// Return the &str trimmed UTF-8 BOM if the input &str contains the BOM.
fn trim_utf8_bom(s: &str) -> &str {
if s.len() > 2 && s.as_bytes().starts_with(&[0xefu8, 0xbbu8, 0xbfu8]) {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer b"\xef\xbb\xbf" here instead of the array

@ryochack
Copy link
Contributor Author

ryochack commented May 3, 2018

Thanks for your advice!
I modified at 31e0984.

@ryankurte ryankurte added the bug label May 3, 2018
@Emilgardis Emilgardis merged commit 425f5ee into rust-embedded:refactor May 3, 2018
@Emilgardis
Copy link
Member

Thank you!
merging without CI as this is still wip

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

Successfully merging this pull request may close these issues.

None yet

3 participants