Skip to content

Added trait implementations to ParseQuantityError#217

Closed
Lucretiel wants to merge 2 commits into
iliekturtles:masterfrom
Lucretiel:error-traits
Closed

Added trait implementations to ParseQuantityError#217
Lucretiel wants to merge 2 commits into
iliekturtles:masterfrom
Lucretiel:error-traits

Conversation

@Lucretiel

Copy link
Copy Markdown
Contributor

No description provided.

@iliekturtles

Copy link
Copy Markdown
Owner

Thanks for the PR! I'm planning to review this weekend.

@iliekturtles

Copy link
Copy Markdown
Owner

Couple minor fixes. If you can review, apply, and then squash your commits I'll merge!

  • use statements can be simplified slightly. uom exposes a lib module to handle std vs. core.
  • UnknownUnit message wording didn't make sense to me. I changed to a possible wording I thought made more sense.
diff --git a/src/lib.rs b/src/lib.rs
index 29115c9..efc91d4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -621,10 +621,7 @@ pub mod fmt {

 /// Unicode string slice manipulation for quantities.
 pub mod str {
-    use core::fmt::{self, Display, Formatter};
-
-    #[cfg(feature = "std")]
-    use std::error::Error;
+    use crate::lib::fmt::{self, Display, Formatter};

     /// Represents an error encountered while parsing a string into a `Quantity`.
     #[allow(missing_copy_implementations)]
@@ -654,11 +651,11 @@ pub mod str {
             match *self {
                 NoSeparator => write!(f, "no space between quantity and units"),
                 ValueParseError => write!(f, "error parsing unit quantity"),
-                UnknownUnit => write!(f, "unrecognized parsing unit of measure"),
+                UnknownUnit => write!(f, "unrecognized unit of measure"),
             }
         }
     }

     #[cfg(feature = "std")]
-    impl Error for ParseQuantityError {}
+    impl crate::lib::error::Error for ParseQuantityError {}
 }

@Lucretiel

Copy link
Copy Markdown
Contributor Author

Thanks for the review! Been busy, will follow up this weekend

@iliekturtles

Copy link
Copy Markdown
Owner

I ended up applying the patch along with assert_all_impl/assert_not_impl_any tests. I'm waiting on build results and will plan on merging unless there was anything else you found.

iliekturtles added a commit that referenced this pull request Nov 25, 2020
Add trait implementations to ParseQuantityError.
@iliekturtles

Copy link
Copy Markdown
Owner

Manually merged. Thanks so much for the PR!

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