From 76a61860991e1e644accc97e0007583034195d06 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Sat, 19 Feb 2022 22:07:15 +0100 Subject: [PATCH 01/11] refactor: remove creating `git_config::values::Value` variants from ut8-s strings --- git-config/src/file/git_config.rs | 4 +- git-config/src/values.rs | 221 +++++++----------- .../file_integeration_test.rs | 4 +- 3 files changed, 82 insertions(+), 147 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index ad8a8ca0b5e..f3a6ae5bb75 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -343,9 +343,9 @@ impl<'event> GitConfig<'event> { /// assert_eq!( /// a_value, /// vec![ - /// Boolean::True(TrueVariant::Explicit(Cow::Borrowed("true"))), + /// Boolean::True(TrueVariant::Explicit(Cow::Borrowed("true".as_bytes()))), /// Boolean::True(TrueVariant::Implicit), - /// Boolean::False(Cow::Borrowed("false")), + /// Boolean::False(Cow::Borrowed("false".as_bytes())), /// ] /// ); /// // ... or explicitly declare the type to avoid the turbofish diff --git a/git-config/src/values.rs b/git-config/src/values.rs index b9d29611218..986ee20a93b 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -162,41 +162,10 @@ impl Value<'_> { } } -// TODO may be remove str handling if not used -impl<'a> From<&'a str> for Value<'a> { - fn from(s: &'a str) -> Self { - if let Ok(bool) = Boolean::try_from(s) { - return Self::Boolean(bool); - } - - if let Ok(int) = Integer::from_str(s) { - return Self::Integer(int); - } - - if let Ok(color) = Color::from_str(s) { - return Self::Color(color); - } - - Self::Other(Cow::Borrowed(s.as_bytes())) - } -} - impl<'a> From<&'a [u8]> for Value<'a> { #[inline] fn from(s: &'a [u8]) -> Self { - // All parsable values must be utf-8 valid - if let Ok(s) = std::str::from_utf8(s) { - Self::from(s) - } else { - Self::Other(Cow::Borrowed(s)) - } - } -} - -impl From for Value<'_> { - #[inline] - fn from(s: String) -> Self { - Self::from(s.into_bytes()) + Self::Other(Cow::Borrowed(s)) } } @@ -526,7 +495,7 @@ impl<'a> From> for Path<'a> { #[allow(missing_docs)] pub enum Boolean<'a> { True(TrueVariant<'a>), - False(Cow<'a, str>), + False(Cow<'a, [u8]>), } impl Boolean<'_> { @@ -549,15 +518,6 @@ impl Boolean<'_> { } } -impl<'a> TryFrom<&'a str> for Boolean<'a> { - type Error = (); - - #[inline] - fn try_from(value: &'a str) -> Result { - Self::try_from(value.as_bytes()) - } -} - impl<'a> TryFrom<&'a [u8]> for Boolean<'a> { type Error = (); @@ -572,21 +532,13 @@ impl<'a> TryFrom<&'a [u8]> for Boolean<'a> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(std::str::from_utf8(value).unwrap().into())); + return Ok(Self::False(value.into())); } Err(()) } } -impl TryFrom for Boolean<'_> { - type Error = String; - - fn try_from(value: String) -> Result { - Self::try_from(value.into_bytes()).map_err(|v| String::from_utf8(v).unwrap()) - } -} - impl TryFrom> for Boolean<'_> { type Error = Vec; @@ -597,7 +549,7 @@ impl TryFrom> for Boolean<'_> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(Cow::Owned(String::from_utf8(value).unwrap()))); + return Ok(Self::False(Cow::Owned(value))); } TrueVariant::try_from(value).map(Self::True) @@ -619,7 +571,8 @@ impl Display for Boolean<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Boolean::True(v) => v.fmt(f), - Boolean::False(v) => write!(f, "{}", v), + // TODO is debug format ok? + Boolean::False(v) => write!(f, "{:?}", v), } } } @@ -639,7 +592,7 @@ impl<'a, 'b: 'a> From<&'b Boolean<'a>> for &'a [u8] { fn from(b: &'b Boolean) -> Self { match b { Boolean::True(t) => t.into(), - Boolean::False(f) => f.as_bytes(), + Boolean::False(f) => f, } } } @@ -677,20 +630,11 @@ impl Serialize for Boolean<'_> { #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] #[allow(missing_docs)] pub enum TrueVariant<'a> { - Explicit(Cow<'a, str>), + Explicit(Cow<'a, [u8]>), /// For values defined without a `= `. Implicit, } -impl<'a> TryFrom<&'a str> for TrueVariant<'a> { - type Error = (); - - #[inline] - fn try_from(value: &'a str) -> Result { - Self::try_from(value.as_bytes()) - } -} - impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { type Error = (); @@ -700,7 +644,7 @@ impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(std::str::from_utf8(value).unwrap().into())) + Ok(Self::Explicit(value.into())) } else if value.is_empty() { Ok(Self::Implicit) } else { @@ -709,15 +653,6 @@ impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { } } -impl TryFrom for TrueVariant<'_> { - type Error = String; - - #[inline] - fn try_from(value: String) -> Result { - Self::try_from(value.into_bytes()).map_err(|v| String::from_utf8(v).unwrap()) - } -} - impl TryFrom> for TrueVariant<'_> { type Error = Vec; @@ -727,7 +662,7 @@ impl TryFrom> for TrueVariant<'_> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(Cow::Owned(String::from_utf8(value).unwrap()))) + Ok(Self::Explicit(Cow::Owned(value))) } else if value.is_empty() { Ok(Self::Implicit) } else { @@ -739,7 +674,8 @@ impl TryFrom> for TrueVariant<'_> { impl Display for TrueVariant<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Self::Explicit(v) = self { - write!(f, "{}", v) + // TODO is debug format ok? + write!(f, "{:?}", v) } else { Ok(()) } @@ -750,7 +686,7 @@ impl<'a, 'b: 'a> From<&'b TrueVariant<'a>> for &'a [u8] { #[inline] fn from(t: &'b TrueVariant<'a>) -> Self { match t { - TrueVariant::Explicit(e) => e.as_bytes(), + TrueVariant::Explicit(e) => e, TrueVariant::Implicit => &[], } } @@ -820,10 +756,12 @@ impl Serialize for Integer { } } -impl FromStr for Integer { - type Err = String; +impl TryFrom<&[u8]> for Integer { + type Error = (); - fn from_str(s: &str) -> Result { + #[inline] + fn try_from(s: &[u8]) -> Result { + let s = std::str::from_utf8(s).map_err(|_| ())?; if let Ok(value) = s.parse() { return Ok(Self { value, suffix: None }); } @@ -831,7 +769,7 @@ impl FromStr for Integer { // Assume we have a prefix at this point. if s.len() <= 1 { - return Err(s.to_string()); + return Err(()); } let (number, suffix) = s.split_at(s.len() - 1); @@ -841,20 +779,11 @@ impl FromStr for Integer { suffix: Some(suffix), }) } else { - Err(s.to_string()) + Err(()) } } } -impl TryFrom<&[u8]> for Integer { - type Error = (); - - #[inline] - fn try_from(s: &[u8]) -> Result { - Self::from_str(std::str::from_utf8(s).map_err(|_| ())?).map_err(|_| ()) - } -} - impl TryFrom> for Integer { type Error = (); @@ -1028,18 +957,12 @@ impl Serialize for Color { } } -/// Discriminating enum for [`Color`] parsing. -pub enum ColorParseError { - /// Too many primary colors were provided. - TooManyColorValues, - /// An invalid color value or attribute was provided. - InvalidColorOption, -} - -impl FromStr for Color { - type Err = ColorParseError; +impl TryFrom<&[u8]> for Color { + type Error = (); - fn from_str(s: &str) -> Result { + #[inline] + fn try_from(s: &[u8]) -> Result { + let s = std::str::from_utf8(s).map_err(|_| ())?; enum ColorItem { Value(ColorValue), Attr(ColorAttribute), @@ -1067,12 +990,12 @@ impl FromStr for Color { } else if new_self.background.is_none() { new_self.background = Some(v); } else { - return Err(ColorParseError::TooManyColorValues); + return Err(()); } } ColorItem::Attr(a) => new_self.attributes.push(a), }, - Err(_) => return Err(ColorParseError::InvalidColorOption), + Err(_) => return Err(()), } } @@ -1080,15 +1003,6 @@ impl FromStr for Color { } } -impl TryFrom<&[u8]> for Color { - type Error = (); - - #[inline] - fn try_from(s: &[u8]) -> Result { - Self::from_str(std::str::from_utf8(s).map_err(|_| ())?).map_err(|_| ()) - } -} - impl TryFrom> for Color { type Error = (); @@ -1426,33 +1340,49 @@ mod normalize { #[cfg(test)] mod boolean { use super::{Boolean, TrueVariant, TryFrom}; + use nom::AsBytes; #[test] fn from_str_false() { - assert_eq!(Boolean::try_from("no"), Ok(Boolean::False("no".into()))); - assert_eq!(Boolean::try_from("off"), Ok(Boolean::False("off".into()))); - assert_eq!(Boolean::try_from("false"), Ok(Boolean::False("false".into()))); - assert_eq!(Boolean::try_from("zero"), Ok(Boolean::False("zero".into()))); - assert_eq!(Boolean::try_from("\"\""), Ok(Boolean::False("\"\"".into()))); + assert_eq!( + Boolean::try_from("no".as_bytes()), + Ok(Boolean::False("no".as_bytes().into())) + ); + assert_eq!( + Boolean::try_from("off".as_bytes()), + Ok(Boolean::False("off".as_bytes().into())) + ); + assert_eq!( + Boolean::try_from("false".as_bytes()), + Ok(Boolean::False("false".as_bytes().into())) + ); + assert_eq!( + Boolean::try_from("zero".as_bytes()), + Ok(Boolean::False("zero".as_bytes().into())) + ); + assert_eq!( + Boolean::try_from("\"\"".as_bytes()), + Ok(Boolean::False("\"\"".as_bytes().into())) + ); } #[test] fn from_str_true() { assert_eq!( - Boolean::try_from("yes"), - Ok(Boolean::True(TrueVariant::Explicit("yes".into()))) + Boolean::try_from("yes".as_bytes()), + Ok(Boolean::True(TrueVariant::Explicit("yes".as_bytes().into()))) ); assert_eq!( - Boolean::try_from("on"), - Ok(Boolean::True(TrueVariant::Explicit("on".into()))) + Boolean::try_from("on".as_bytes()), + Ok(Boolean::True(TrueVariant::Explicit("on".as_bytes().into()))) ); assert_eq!( - Boolean::try_from("true"), - Ok(Boolean::True(TrueVariant::Explicit("true".into()))) + Boolean::try_from("true".as_bytes()), + Ok(Boolean::True(TrueVariant::Explicit("true".as_bytes().into()))) ); assert_eq!( - Boolean::try_from("one"), - Ok(Boolean::True(TrueVariant::Explicit("one".into()))) + Boolean::try_from("one".as_bytes()), + Ok(Boolean::True(TrueVariant::Explicit("one".as_bytes().into()))) ); } @@ -1460,29 +1390,34 @@ mod boolean { fn ignores_case() { // Random subset for word in &["no", "yes", "off", "true", "zero"] { - let first: bool = Boolean::try_from(*word).unwrap().into(); - let second: bool = Boolean::try_from(&*word.to_uppercase()).unwrap().into(); + let first: bool = Boolean::try_from(word.as_bytes()).unwrap().into(); + let second: bool = Boolean::try_from(&*word.to_uppercase().as_bytes()).unwrap().into(); assert_eq!(first, second); } } #[test] fn from_str_err() { - assert!(Boolean::try_from("yesn't").is_err()); - assert!(Boolean::try_from("yesno").is_err()); + assert!(Boolean::try_from("yesn't".as_bytes()).is_err()); + assert!(Boolean::try_from("yesno".as_bytes()).is_err()); } } #[cfg(test)] mod integer { - use super::{FromStr, Integer, IntegerSuffix}; + use super::{Integer, IntegerSuffix}; + use nom::AsBytes; + use std::convert::TryFrom; #[test] fn from_str_no_suffix() { - assert_eq!(Integer::from_str("1").unwrap(), Integer { value: 1, suffix: None }); + assert_eq!( + Integer::try_from("1".as_bytes()).unwrap(), + Integer { value: 1, suffix: None } + ); assert_eq!( - Integer::from_str("-1").unwrap(), + Integer::try_from("-1".as_bytes()).unwrap(), Integer { value: -1, suffix: None @@ -1493,7 +1428,7 @@ mod integer { #[test] fn from_str_with_suffix() { assert_eq!( - Integer::from_str("1k").unwrap(), + Integer::try_from("1k".as_bytes()).unwrap(), Integer { value: 1, suffix: Some(IntegerSuffix::Kibi), @@ -1501,7 +1436,7 @@ mod integer { ); assert_eq!( - Integer::from_str("1m").unwrap(), + Integer::try_from("1m".as_bytes()).unwrap(), Integer { value: 1, suffix: Some(IntegerSuffix::Mebi), @@ -1509,7 +1444,7 @@ mod integer { ); assert_eq!( - Integer::from_str("1g").unwrap(), + Integer::try_from("1g".as_bytes()).unwrap(), Integer { value: 1, suffix: Some(IntegerSuffix::Gibi), @@ -1519,13 +1454,13 @@ mod integer { #[test] fn invalid_from_str() { - assert!(Integer::from_str("").is_err()); - assert!(Integer::from_str("-").is_err()); - assert!(Integer::from_str("k").is_err()); - assert!(Integer::from_str("m").is_err()); - assert!(Integer::from_str("g").is_err()); - assert!(Integer::from_str("123123123123123123123123").is_err()); - assert!(Integer::from_str("gg").is_err()); + assert!(Integer::try_from("".as_bytes()).is_err()); + assert!(Integer::try_from("-".as_bytes()).is_err()); + assert!(Integer::try_from("k".as_bytes()).is_err()); + assert!(Integer::try_from("m".as_bytes()).is_err()); + assert!(Integer::try_from("g".as_bytes()).is_err()); + assert!(Integer::try_from("123123123123123123123123".as_bytes()).is_err()); + assert!(Integer::try_from("gg".as_bytes()).is_err()); } } diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index eb10ee7a195..c90d2399956 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -31,7 +31,7 @@ fn get_value_for_all_provided_values() -> crate::Result { assert_eq!( file.value::("core", None, "bool-explicit")?, - Boolean::False(Cow::Borrowed("false")) + Boolean::False(Cow::Borrowed("false".as_bytes())) ); assert_eq!( @@ -111,7 +111,7 @@ fn get_value_looks_up_all_sections_before_failing() -> crate::Result { assert_eq!( file.value::("core", None, "bool-explicit")?, - Boolean::False(Cow::Borrowed("false")) + Boolean::False(Cow::Borrowed("false".as_bytes())) ); Ok(()) From 842ad3733aacab28289a3ebbccd6a7729fba8da6 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Sat, 19 Feb 2022 22:11:37 +0100 Subject: [PATCH 02/11] refactor: rename Other to Bytes in `git_config::values::Value` --- git-config/src/file/git_config.rs | 6 +++--- git-config/src/values.rs | 15 +++++++-------- .../integration_tests/file_integeration_test.rs | 2 +- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index f3a6ae5bb75..f563d973e47 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -350,7 +350,7 @@ impl<'event> GitConfig<'event> { /// ); /// // ... or explicitly declare the type to avoid the turbofish /// let c_value: Vec = git_config.multi_value("core", None, "c")?; - /// assert_eq!(c_value, vec![Value::Other(Cow::Borrowed(b"g"))]); + /// assert_eq!(c_value, vec![Value::Bytes(Cow::Borrowed(b"g"))]); /// # Ok::<(), GitConfigError>(()) /// ``` /// @@ -2102,7 +2102,7 @@ mod get_value { let first_value: Value = config.value("core", None, "a")?; let second_value: Boolean = config.value("core", None, "c")?; - assert_eq!(first_value, Value::Other(Cow::Borrowed(b"b"))); + assert_eq!(first_value, Value::Bytes(Cow::Borrowed(b"b"))); assert_eq!(second_value, Boolean::True(TrueVariant::Implicit)); Ok(()) @@ -2123,7 +2123,7 @@ mod get_value { let config = GitConfig::try_from(config).unwrap(); let value = config.value::("remote", Some("origin"), "url").unwrap(); - assert_eq!(value, Value::Other(Cow::Borrowed(b"git@github.com:Byron/gitoxide.git"))); + assert_eq!(value, Value::Bytes(Cow::Borrowed(b"git@github.com:Byron/gitoxide.git"))); } } diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 986ee20a93b..91355df69a0 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -146,9 +146,8 @@ pub enum Value<'a> { Integer(Integer), Color(Color), /// If a value does not match from any of the other variants, then this - /// variant will be matched. As a result, conversion from a `str`-like item - /// will never fail. - Other(Cow<'a, [u8]>), + /// variant will be matched. + Bytes(Cow<'a, [u8]>), } impl Value<'_> { @@ -165,7 +164,7 @@ impl Value<'_> { impl<'a> From<&'a [u8]> for Value<'a> { #[inline] fn from(s: &'a [u8]) -> Self { - Self::Other(Cow::Borrowed(s)) + Self::Bytes(Cow::Borrowed(s)) } } @@ -179,7 +178,7 @@ impl From> for Value<'_> { return Self::Color(color); } - Boolean::try_from(s).map_or_else(|v| Self::Other(Cow::Owned(v)), Self::Boolean) + Boolean::try_from(s).map_or_else(|v| Self::Bytes(Cow::Owned(v)), Self::Boolean) } } @@ -207,7 +206,7 @@ impl From<&Value<'_>> for Vec { Value::Boolean(b) => b.into(), Value::Integer(i) => i.into(), Value::Color(c) => c.into(), - Value::Other(o) => o.to_vec(), + Value::Bytes(o) => o.to_vec(), } } } @@ -220,7 +219,7 @@ impl Display for Value<'_> { Value::Boolean(b) => b.fmt(f), Value::Integer(i) => i.fmt(f), Value::Color(c) => c.fmt(f), - Value::Other(o) => match std::str::from_utf8(o) { + Value::Bytes(o) => match std::str::from_utf8(o) { Ok(v) => v.fmt(f), Err(_) => write!(f, "{:?}", o), }, @@ -238,7 +237,7 @@ impl Serialize for Value<'_> { Value::Boolean(b) => b.serialize(serializer), Value::Integer(i) => i.serialize(serializer), Value::Color(c) => c.serialize(serializer), - Value::Other(i) => i.serialize(serializer), + Value::Bytes(i) => i.serialize(serializer), } } } diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index c90d2399956..91234479eda 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -74,7 +74,7 @@ fn get_value_for_all_provided_values() -> crate::Result { assert_eq!( file.value::("core", None, "other")?, - Value::Other(Cow::Borrowed(b"hello world")) + Value::Bytes(Cow::Borrowed(b"hello world")) ); let actual = file.value::("core", None, "location")?; From 9a89c6cfcfd03cf1ef7118d1827087b48a023b75 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Mon, 21 Feb 2022 20:38:28 +0100 Subject: [PATCH 03/11] Use string to byte slice function. --- git-config/src/values.rs | 92 +++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 91355df69a0..34312a325a5 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -138,6 +138,11 @@ pub fn normalize_str(input: &str) -> Cow<'_, [u8]> { normalize_bytes(input.as_bytes()) } +/// Converts string to byte slice +fn b(s: &str) -> &[u8] { + s.as_bytes() +} + /// Fully enumerated valid types that a `git-config` value can be. #[allow(missing_docs)] #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] @@ -353,6 +358,7 @@ pub mod path { mod interpolate_tests { use std::borrow::Cow; + use crate::values::b; use crate::values::{path::interpolate::Error, Path}; #[test] @@ -369,7 +375,7 @@ pub mod path { #[test] fn empty_path_is_error() { assert!(matches!( - Path::from(Cow::Borrowed("".as_bytes())).interpolate(None), + Path::from(Cow::Borrowed(b(""))).interpolate(None), Err(Error::Missing { what: "path" }) )); } @@ -403,7 +409,7 @@ pub mod path { let path = "./%(prefix)/foo/bar"; let git_install_dir = "/tmp/git"; assert_eq!( - Path::from(Cow::Borrowed(path.as_bytes())) + Path::from(Cow::Borrowed(b(path))) .interpolate(Some(std::path::Path::new(git_install_dir))) .unwrap(), std::path::Path::new(path) @@ -445,7 +451,7 @@ pub mod path { let path = format!("{}{}{}", specific_user_home, std::path::MAIN_SEPARATOR, path_suffix); let expected = format!("{}{}{}", home, std::path::MAIN_SEPARATOR, path_suffix); assert_eq!( - Path::from(Cow::Borrowed(path.as_bytes())).interpolate(None).unwrap(), + Path::from(Cow::Borrowed(b(&path))).interpolate(None).unwrap(), std::path::Path::new(&expected), "it keeps path separators as is" ); @@ -1339,49 +1345,34 @@ mod normalize { #[cfg(test)] mod boolean { use super::{Boolean, TrueVariant, TryFrom}; - use nom::AsBytes; + use crate::values::b; #[test] fn from_str_false() { - assert_eq!( - Boolean::try_from("no".as_bytes()), - Ok(Boolean::False("no".as_bytes().into())) - ); - assert_eq!( - Boolean::try_from("off".as_bytes()), - Ok(Boolean::False("off".as_bytes().into())) - ); - assert_eq!( - Boolean::try_from("false".as_bytes()), - Ok(Boolean::False("false".as_bytes().into())) - ); - assert_eq!( - Boolean::try_from("zero".as_bytes()), - Ok(Boolean::False("zero".as_bytes().into())) - ); - assert_eq!( - Boolean::try_from("\"\"".as_bytes()), - Ok(Boolean::False("\"\"".as_bytes().into())) - ); + assert_eq!(Boolean::try_from(b("no")), Ok(Boolean::False(b("no").into()))); + assert_eq!(Boolean::try_from(b("off")), Ok(Boolean::False(b("off").into()))); + assert_eq!(Boolean::try_from(b("false")), Ok(Boolean::False(b("false").into()))); + assert_eq!(Boolean::try_from(b("zero")), Ok(Boolean::False(b("zero").into()))); + assert_eq!(Boolean::try_from(b("\"\"")), Ok(Boolean::False(b("\"\"").into()))); } #[test] fn from_str_true() { assert_eq!( - Boolean::try_from("yes".as_bytes()), - Ok(Boolean::True(TrueVariant::Explicit("yes".as_bytes().into()))) + Boolean::try_from(b("yes")), + Ok(Boolean::True(TrueVariant::Explicit(b("yes").into()))) ); assert_eq!( - Boolean::try_from("on".as_bytes()), - Ok(Boolean::True(TrueVariant::Explicit("on".as_bytes().into()))) + Boolean::try_from(b("on")), + Ok(Boolean::True(TrueVariant::Explicit(b("on").into()))) ); assert_eq!( - Boolean::try_from("true".as_bytes()), - Ok(Boolean::True(TrueVariant::Explicit("true".as_bytes().into()))) + Boolean::try_from(b("true")), + Ok(Boolean::True(TrueVariant::Explicit(b("true").into()))) ); assert_eq!( - Boolean::try_from("one".as_bytes()), - Ok(Boolean::True(TrueVariant::Explicit("one".as_bytes().into()))) + Boolean::try_from(b("one")), + Ok(Boolean::True(TrueVariant::Explicit(b("one").into()))) ); } @@ -1389,34 +1380,31 @@ mod boolean { fn ignores_case() { // Random subset for word in &["no", "yes", "off", "true", "zero"] { - let first: bool = Boolean::try_from(word.as_bytes()).unwrap().into(); - let second: bool = Boolean::try_from(&*word.to_uppercase().as_bytes()).unwrap().into(); + let first: bool = Boolean::try_from(b(word)).unwrap().into(); + let second: bool = Boolean::try_from(b(&*word.to_uppercase())).unwrap().into(); assert_eq!(first, second); } } #[test] fn from_str_err() { - assert!(Boolean::try_from("yesn't".as_bytes()).is_err()); - assert!(Boolean::try_from("yesno".as_bytes()).is_err()); + assert!(Boolean::try_from(b("yesn't")).is_err()); + assert!(Boolean::try_from(b("yesno")).is_err()); } } #[cfg(test)] mod integer { use super::{Integer, IntegerSuffix}; - use nom::AsBytes; + use crate::values::b; use std::convert::TryFrom; #[test] fn from_str_no_suffix() { - assert_eq!( - Integer::try_from("1".as_bytes()).unwrap(), - Integer { value: 1, suffix: None } - ); + assert_eq!(Integer::try_from(b("1")).unwrap(), Integer { value: 1, suffix: None }); assert_eq!( - Integer::try_from("-1".as_bytes()).unwrap(), + Integer::try_from(b("-1")).unwrap(), Integer { value: -1, suffix: None @@ -1427,7 +1415,7 @@ mod integer { #[test] fn from_str_with_suffix() { assert_eq!( - Integer::try_from("1k".as_bytes()).unwrap(), + Integer::try_from(b("1k")).unwrap(), Integer { value: 1, suffix: Some(IntegerSuffix::Kibi), @@ -1435,7 +1423,7 @@ mod integer { ); assert_eq!( - Integer::try_from("1m".as_bytes()).unwrap(), + Integer::try_from(b("1m")).unwrap(), Integer { value: 1, suffix: Some(IntegerSuffix::Mebi), @@ -1443,7 +1431,7 @@ mod integer { ); assert_eq!( - Integer::try_from("1g".as_bytes()).unwrap(), + Integer::try_from(b("1g")).unwrap(), Integer { value: 1, suffix: Some(IntegerSuffix::Gibi), @@ -1453,13 +1441,13 @@ mod integer { #[test] fn invalid_from_str() { - assert!(Integer::try_from("".as_bytes()).is_err()); - assert!(Integer::try_from("-".as_bytes()).is_err()); - assert!(Integer::try_from("k".as_bytes()).is_err()); - assert!(Integer::try_from("m".as_bytes()).is_err()); - assert!(Integer::try_from("g".as_bytes()).is_err()); - assert!(Integer::try_from("123123123123123123123123".as_bytes()).is_err()); - assert!(Integer::try_from("gg".as_bytes()).is_err()); + assert!(Integer::try_from(b("")).is_err()); + assert!(Integer::try_from(b("-")).is_err()); + assert!(Integer::try_from(b("k")).is_err()); + assert!(Integer::try_from(b("m")).is_err()); + assert!(Integer::try_from(b("g")).is_err()); + assert!(Integer::try_from(b("123123123123123123123123")).is_err()); + assert!(Integer::try_from(b("gg")).is_err()); } } From b28cb1b1e307a3a5be12d75a77728171cf4118a7 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Mon, 21 Feb 2022 20:58:56 +0100 Subject: [PATCH 04/11] Change back TrueVariant::Explicit to str. --- git-config/src/file/git_config.rs | 2 +- git-config/src/values.rs | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index f563d973e47..2bfe47a491c 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -343,7 +343,7 @@ impl<'event> GitConfig<'event> { /// assert_eq!( /// a_value, /// vec![ - /// Boolean::True(TrueVariant::Explicit(Cow::Borrowed("true".as_bytes()))), + /// Boolean::True(TrueVariant::Explicit(Cow::Borrowed("true"))), /// Boolean::True(TrueVariant::Implicit), /// Boolean::False(Cow::Borrowed("false".as_bytes())), /// ] diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 34312a325a5..5556fcbae8b 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -635,7 +635,7 @@ impl Serialize for Boolean<'_> { #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] #[allow(missing_docs)] pub enum TrueVariant<'a> { - Explicit(Cow<'a, [u8]>), + Explicit(Cow<'a, str>), /// For values defined without a `= `. Implicit, } @@ -649,7 +649,7 @@ impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(value.into())) + Ok(Self::Explicit(std::str::from_utf8(value).unwrap().into())) } else if value.is_empty() { Ok(Self::Implicit) } else { @@ -667,7 +667,7 @@ impl TryFrom> for TrueVariant<'_> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(Cow::Owned(value))) + Ok(Self::Explicit(Cow::Owned(String::from_utf8(value).unwrap()))) } else if value.is_empty() { Ok(Self::Implicit) } else { @@ -680,7 +680,7 @@ impl Display for TrueVariant<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Self::Explicit(v) = self { // TODO is debug format ok? - write!(f, "{:?}", v) + write!(f, "{}", v) } else { Ok(()) } @@ -691,7 +691,7 @@ impl<'a, 'b: 'a> From<&'b TrueVariant<'a>> for &'a [u8] { #[inline] fn from(t: &'b TrueVariant<'a>) -> Self { match t { - TrueVariant::Explicit(e) => e, + TrueVariant::Explicit(e) => e.as_bytes(), TrueVariant::Implicit => &[], } } @@ -1360,19 +1360,19 @@ mod boolean { fn from_str_true() { assert_eq!( Boolean::try_from(b("yes")), - Ok(Boolean::True(TrueVariant::Explicit(b("yes").into()))) + Ok(Boolean::True(TrueVariant::Explicit("yes".into()))) ); assert_eq!( Boolean::try_from(b("on")), - Ok(Boolean::True(TrueVariant::Explicit(b("on").into()))) + Ok(Boolean::True(TrueVariant::Explicit("on".into()))) ); assert_eq!( Boolean::try_from(b("true")), - Ok(Boolean::True(TrueVariant::Explicit(b("true").into()))) + Ok(Boolean::True(TrueVariant::Explicit("true".into()))) ); assert_eq!( Boolean::try_from(b("one")), - Ok(Boolean::True(TrueVariant::Explicit(b("one").into()))) + Ok(Boolean::True(TrueVariant::Explicit("one".into()))) ); } From f12e667b8a6bfbc7697f9d3fb55ca4d9b339fd43 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Mon, 21 Feb 2022 21:11:01 +0100 Subject: [PATCH 05/11] Change back `Boolean::False` to str. --- git-config/src/file/git_config.rs | 2 +- git-config/src/values.rs | 20 +++++++++---------- .../file_integeration_test.rs | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index 2bfe47a491c..411a844f5a3 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -345,7 +345,7 @@ impl<'event> GitConfig<'event> { /// vec![ /// Boolean::True(TrueVariant::Explicit(Cow::Borrowed("true"))), /// Boolean::True(TrueVariant::Implicit), - /// Boolean::False(Cow::Borrowed("false".as_bytes())), + /// Boolean::False(Cow::Borrowed("false")), /// ] /// ); /// // ... or explicitly declare the type to avoid the turbofish diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 5556fcbae8b..cadac084ded 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -500,7 +500,7 @@ impl<'a> From> for Path<'a> { #[allow(missing_docs)] pub enum Boolean<'a> { True(TrueVariant<'a>), - False(Cow<'a, [u8]>), + False(Cow<'a, str>), } impl Boolean<'_> { @@ -537,7 +537,7 @@ impl<'a> TryFrom<&'a [u8]> for Boolean<'a> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(value.into())); + return Ok(Self::False(std::str::from_utf8(value).unwrap().into())); } Err(()) @@ -554,7 +554,7 @@ impl TryFrom> for Boolean<'_> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(Cow::Owned(value))); + return Ok(Self::False(Cow::Owned(String::from_utf8(value).unwrap()))); } TrueVariant::try_from(value).map(Self::True) @@ -577,7 +577,7 @@ impl Display for Boolean<'_> { match self { Boolean::True(v) => v.fmt(f), // TODO is debug format ok? - Boolean::False(v) => write!(f, "{:?}", v), + Boolean::False(v) => write!(f, "{}", v), } } } @@ -597,7 +597,7 @@ impl<'a, 'b: 'a> From<&'b Boolean<'a>> for &'a [u8] { fn from(b: &'b Boolean) -> Self { match b { Boolean::True(t) => t.into(), - Boolean::False(f) => f, + Boolean::False(f) => f.as_bytes(), } } } @@ -1349,11 +1349,11 @@ mod boolean { #[test] fn from_str_false() { - assert_eq!(Boolean::try_from(b("no")), Ok(Boolean::False(b("no").into()))); - assert_eq!(Boolean::try_from(b("off")), Ok(Boolean::False(b("off").into()))); - assert_eq!(Boolean::try_from(b("false")), Ok(Boolean::False(b("false").into()))); - assert_eq!(Boolean::try_from(b("zero")), Ok(Boolean::False(b("zero").into()))); - assert_eq!(Boolean::try_from(b("\"\"")), Ok(Boolean::False(b("\"\"").into()))); + assert_eq!(Boolean::try_from(b("no")), Ok(Boolean::False("no".into()))); + assert_eq!(Boolean::try_from(b("off")), Ok(Boolean::False("off".into()))); + assert_eq!(Boolean::try_from(b("false")), Ok(Boolean::False("false".into()))); + assert_eq!(Boolean::try_from(b("zero")), Ok(Boolean::False("zero".into()))); + assert_eq!(Boolean::try_from(b("\"\"")), Ok(Boolean::False("\"\"".into()))); } #[test] diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 91234479eda..94e4636232e 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -31,7 +31,7 @@ fn get_value_for_all_provided_values() -> crate::Result { assert_eq!( file.value::("core", None, "bool-explicit")?, - Boolean::False(Cow::Borrowed("false".as_bytes())) + Boolean::False(Cow::Borrowed("false")) ); assert_eq!( @@ -111,7 +111,7 @@ fn get_value_looks_up_all_sections_before_failing() -> crate::Result { assert_eq!( file.value::("core", None, "bool-explicit")?, - Boolean::False(Cow::Borrowed("false".as_bytes())) + Boolean::False(Cow::Borrowed("false")) ); Ok(()) From 951b304da4921535eba5f64cfbc585e7a0a070df Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Mon, 21 Feb 2022 21:11:56 +0100 Subject: [PATCH 06/11] Change back `Boolean::False` to str. --- git-config/src/values.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/git-config/src/values.rs b/git-config/src/values.rs index cadac084ded..ee7c00be5bc 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -576,7 +576,6 @@ impl Display for Boolean<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Boolean::True(v) => v.fmt(f), - // TODO is debug format ok? Boolean::False(v) => write!(f, "{}", v), } } @@ -679,7 +678,6 @@ impl TryFrom> for TrueVariant<'_> { impl Display for TrueVariant<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if let Self::Explicit(v) = self { - // TODO is debug format ok? write!(f, "{}", v) } else { Ok(()) From 2176dc6421d5be58631bee251bf0371b0fd45402 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Tue, 22 Feb 2022 20:58:37 +0100 Subject: [PATCH 07/11] Value is struct of bytes. --- git-config/src/file/git_config.rs | 20 +++-- git-config/src/values.rs | 73 ++----------------- .../file_integeration_test.rs | 4 +- 3 files changed, 26 insertions(+), 71 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index 411a844f5a3..ed67f5ea6b4 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -277,7 +277,7 @@ impl<'event> GitConfig<'event> { /// /// ``` /// # use git_config::file::{GitConfig, GitConfigError}; - /// # use git_config::values::{Integer, Value, Boolean}; + /// # use git_config::values::{Integer, Boolean}; /// # use std::borrow::Cow; /// # use std::convert::TryFrom; /// let config = r#" @@ -350,7 +350,7 @@ impl<'event> GitConfig<'event> { /// ); /// // ... or explicitly declare the type to avoid the turbofish /// let c_value: Vec = git_config.multi_value("core", None, "c")?; - /// assert_eq!(c_value, vec![Value::Bytes(Cow::Borrowed(b"g"))]); + /// assert_eq!(c_value, vec![Value { value: Cow::Borrowed(b"g") }]); /// # Ok::<(), GitConfigError>(()) /// ``` /// @@ -436,7 +436,7 @@ impl<'event> GitConfig<'event> { /// /// ``` /// # use git_config::file::{GitConfig, GitConfigError}; - /// # use git_config::values::{Integer, Value, Boolean, TrueVariant}; + /// # use git_config::values::{Integer, Boolean, TrueVariant}; /// # use std::borrow::Cow; /// # use std::convert::TryFrom; /// let config = r#" @@ -2102,7 +2102,12 @@ mod get_value { let first_value: Value = config.value("core", None, "a")?; let second_value: Boolean = config.value("core", None, "c")?; - assert_eq!(first_value, Value::Bytes(Cow::Borrowed(b"b"))); + assert_eq!( + first_value, + Value { + value: Cow::Borrowed(b"b") + } + ); assert_eq!(second_value, Boolean::True(TrueVariant::Implicit)); Ok(()) @@ -2123,7 +2128,12 @@ mod get_value { let config = GitConfig::try_from(config).unwrap(); let value = config.value::("remote", Some("origin"), "url").unwrap(); - assert_eq!(value, Value::Bytes(Cow::Borrowed(b"git@github.com:Byron/gitoxide.git"))); + assert_eq!( + value, + Value { + value: Cow::Borrowed(b"git@github.com:Byron/gitoxide.git") + } + ); } } diff --git a/git-config/src/values.rs b/git-config/src/values.rs index ee7c00be5bc..170e324d87d 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -143,47 +143,25 @@ fn b(s: &str) -> &[u8] { s.as_bytes() } -/// Fully enumerated valid types that a `git-config` value can be. -#[allow(missing_docs)] +/// Any string value #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -pub enum Value<'a> { - Boolean(Boolean<'a>), - Integer(Integer), - Color(Color), - /// If a value does not match from any of the other variants, then this - /// variant will be matched. - Bytes(Cow<'a, [u8]>), -} - -impl Value<'_> { - /// Generates a byte representation of the value. This should be used when - /// non-UTF-8 sequences are present or a UTF-8 representation can't be - /// guaranteed. - #[inline] - #[must_use] - pub fn to_vec(&self) -> Vec { - self.into() - } +pub struct Value<'a> { + /// bytes + pub value: Cow<'a, [u8]>, } impl<'a> From<&'a [u8]> for Value<'a> { #[inline] fn from(s: &'a [u8]) -> Self { - Self::Bytes(Cow::Borrowed(s)) + Self { + value: Cow::Borrowed(s), + } } } impl From> for Value<'_> { fn from(s: Vec) -> Self { - if let Ok(int) = Integer::try_from(s.as_ref()) { - return Self::Integer(int); - } - - if let Ok(color) = Color::try_from(s.as_ref()) { - return Self::Color(color); - } - - Boolean::try_from(s).map_or_else(|v| Self::Bytes(Cow::Owned(v)), Self::Boolean) + Self { value: Cow::Owned(s) } } } @@ -197,41 +175,6 @@ impl<'a> From> for Value<'a> { } } -impl From> for Vec { - #[inline] - fn from(v: Value) -> Self { - v.into() - } -} - -impl From<&Value<'_>> for Vec { - #[inline] - fn from(v: &Value) -> Self { - match v { - Value::Boolean(b) => b.into(), - Value::Integer(i) => i.into(), - Value::Color(c) => c.into(), - Value::Bytes(o) => o.to_vec(), - } - } -} - -impl Display for Value<'_> { - /// Note that this is a best-effort attempt at printing a `Value`. If there - /// are non UTF-8 values in your config, this will _NOT_ render as read. - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Value::Boolean(b) => b.fmt(f), - Value::Integer(i) => i.fmt(f), - Value::Color(c) => c.fmt(f), - Value::Bytes(o) => match std::str::from_utf8(o) { - Ok(v) => v.fmt(f), - Err(_) => write!(f, "{:?}", o), - }, - } - } -} - #[cfg(feature = "serde")] impl Serialize for Value<'_> { fn serialize(&self, serializer: S) -> Result diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 94e4636232e..410f54f2d14 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -74,7 +74,9 @@ fn get_value_for_all_provided_values() -> crate::Result { assert_eq!( file.value::("core", None, "other")?, - Value::Bytes(Cow::Borrowed(b"hello world")) + Value { + value: Cow::Borrowed(b"hello world") + } ); let actual = file.value::("core", None, "location")?; From 1b833c8c37e1db21f3b28a55e575f79456b127ff Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Tue, 22 Feb 2022 22:28:19 +0100 Subject: [PATCH 08/11] Use quick_error. --- git-config/src/file/git_config.rs | 16 +- git-config/src/values.rs | 154 ++++++++++++------ .../file_integeration_test.rs | 4 +- 3 files changed, 115 insertions(+), 59 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index ed67f5ea6b4..f6b2cd38f83 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -326,7 +326,7 @@ impl<'event> GitConfig<'event> { /// /// ``` /// # use git_config::file::{GitConfig, GitConfigError}; - /// # use git_config::values::{Integer, Value, Boolean, TrueVariant}; + /// # use git_config::values::{Integer, Bytes, Boolean, TrueVariant}; /// # use std::borrow::Cow; /// # use std::convert::TryFrom; /// let config = r#" @@ -349,8 +349,8 @@ impl<'event> GitConfig<'event> { /// ] /// ); /// // ... or explicitly declare the type to avoid the turbofish - /// let c_value: Vec = git_config.multi_value("core", None, "c")?; - /// assert_eq!(c_value, vec![Value { value: Cow::Borrowed(b"g") }]); + /// let c_value: Vec = git_config.multi_value("core", None, "c")?; + /// assert_eq!(c_value, vec![Bytes { value: Cow::Borrowed(b"g") }]); /// # Ok::<(), GitConfigError>(()) /// ``` /// @@ -2094,17 +2094,17 @@ mod get_value { use std::error::Error; use super::{Cow, GitConfig, TryFrom}; - use crate::values::{Boolean, TrueVariant, Value}; + use crate::values::{Boolean, Bytes, TrueVariant}; #[test] fn single_section() -> Result<(), Box> { let config = GitConfig::try_from("[core]\na=b\nc").unwrap(); - let first_value: Value = config.value("core", None, "a")?; + let first_value: Bytes = config.value("core", None, "a")?; let second_value: Boolean = config.value("core", None, "c")?; assert_eq!( first_value, - Value { + Bytes { value: Cow::Borrowed(b"b") } ); @@ -2127,10 +2127,10 @@ mod get_value { "#; let config = GitConfig::try_from(config).unwrap(); - let value = config.value::("remote", Some("origin"), "url").unwrap(); + let value = config.value::("remote", Some("origin"), "url").unwrap(); assert_eq!( value, - Value { + Bytes { value: Cow::Borrowed(b"git@github.com:Byron/gitoxide.git") } ); diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 170e324d87d..66a843eef57 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -1,5 +1,6 @@ //! Rust containers for valid `git-config` types. +use quick_error::quick_error; use std::{borrow::Cow, convert::TryFrom, fmt::Display, str::FromStr}; #[cfg(feature = "serde")] @@ -145,12 +146,12 @@ fn b(s: &str) -> &[u8] { /// Any string value #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -pub struct Value<'a> { +pub struct Bytes<'a> { /// bytes pub value: Cow<'a, [u8]>, } -impl<'a> From<&'a [u8]> for Value<'a> { +impl<'a> From<&'a [u8]> for Bytes<'a> { #[inline] fn from(s: &'a [u8]) -> Self { Self { @@ -159,13 +160,13 @@ impl<'a> From<&'a [u8]> for Value<'a> { } } -impl From> for Value<'_> { +impl From> for Bytes<'_> { fn from(s: Vec) -> Self { Self { value: Cow::Owned(s) } } } -impl<'a> From> for Value<'a> { +impl<'a> From> for Bytes<'a> { #[inline] fn from(c: Cow<'a, [u8]>) -> Self { match c { @@ -182,10 +183,7 @@ impl Serialize for Value<'_> { S: serde::Serializer, { match self { - Value::Boolean(b) => b.serialize(serializer), - Value::Integer(i) => i.serialize(serializer), - Value::Color(c) => c.serialize(serializer), - Value::Bytes(i) => i.serialize(serializer), + Value(i) => i.serialize(serializer), } } } @@ -466,8 +464,25 @@ impl Boolean<'_> { } } +quick_error! { + #[derive(Debug, PartialEq)] + /// The error returned when creating `Boolean` from byte string. + #[allow(missing_docs)] + pub enum BooleanError { + Utf8Conversion(err: std::str::Utf8Error) { + display("Ill-formed UTF-8") + source(err) + from() + from(err: std::string::FromUtf8Error) -> (err.utf8_error()) + } + InvalidFormat { + display("Invalid argument format") + } + } +} + impl<'a> TryFrom<&'a [u8]> for Boolean<'a> { - type Error = (); + type Error = BooleanError; fn try_from(value: &'a [u8]) -> Result { if let Ok(v) = TrueVariant::try_from(value) { @@ -480,15 +495,15 @@ impl<'a> TryFrom<&'a [u8]> for Boolean<'a> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(std::str::from_utf8(value).unwrap().into())); + return Ok(Self::False(std::str::from_utf8(value)?.into())); } - Err(()) + Err(BooleanError::InvalidFormat) } } impl TryFrom> for Boolean<'_> { - type Error = Vec; + type Error = BooleanError; fn try_from(value: Vec) -> Result { if value.eq_ignore_ascii_case(b"no") @@ -497,7 +512,7 @@ impl TryFrom> for Boolean<'_> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(Cow::Owned(String::from_utf8(value).unwrap()))); + return Ok(Self::False(Cow::Owned(String::from_utf8(value)?))); } TrueVariant::try_from(value).map(Self::True) @@ -505,11 +520,11 @@ impl TryFrom> for Boolean<'_> { } impl<'a> TryFrom> for Boolean<'a> { - type Error = (); + type Error = BooleanError; fn try_from(c: Cow<'a, [u8]>) -> Result { match c { Cow::Borrowed(c) => Self::try_from(c), - Cow::Owned(c) => Self::try_from(c).map_err(|_| ()), + Cow::Owned(c) => Self::try_from(c), } } } @@ -583,7 +598,7 @@ pub enum TrueVariant<'a> { } impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { - type Error = (); + type Error = BooleanError; fn try_from(value: &'a [u8]) -> Result { if value.eq_ignore_ascii_case(b"yes") @@ -591,17 +606,17 @@ impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(std::str::from_utf8(value).unwrap().into())) + Ok(Self::Explicit(std::str::from_utf8(value)?.into())) } else if value.is_empty() { Ok(Self::Implicit) } else { - Err(()) + Err(BooleanError::InvalidFormat) } } } impl TryFrom> for TrueVariant<'_> { - type Error = Vec; + type Error = BooleanError; fn try_from(value: Vec) -> Result { if value.eq_ignore_ascii_case(b"yes") @@ -609,11 +624,11 @@ impl TryFrom> for TrueVariant<'_> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(Cow::Owned(String::from_utf8(value).unwrap()))) + Ok(Self::Explicit(Cow::Owned(String::from_utf8(value)?))) } else if value.is_empty() { Ok(Self::Implicit) } else { - Err(value) + Err(BooleanError::InvalidFormat) } } } @@ -702,12 +717,34 @@ impl Serialize for Integer { } } +quick_error! { + #[derive(Debug)] + /// The error returned when creating `Integer` from byte string. + #[allow(missing_docs)] + pub enum IntegerError { + Utf8Conversion(err: std::str::Utf8Error) { + display("Ill-formed UTF-8") + source(err) + from() + } + MissingPrefix { + display("Missing prefix") + } + InvalidFormat { + display("Invalid argument format") + } + InvalidSuffix { + display("Invalid suffix") + } + } +} + impl TryFrom<&[u8]> for Integer { - type Error = (); + type Error = IntegerError; #[inline] fn try_from(s: &[u8]) -> Result { - let s = std::str::from_utf8(s).map_err(|_| ())?; + let s = std::str::from_utf8(s)?; if let Ok(value) = s.parse() { return Ok(Self { value, suffix: None }); } @@ -715,7 +752,7 @@ impl TryFrom<&[u8]> for Integer { // Assume we have a prefix at this point. if s.len() <= 1 { - return Err(()); + return Err(IntegerError::MissingPrefix); } let (number, suffix) = s.split_at(s.len() - 1); @@ -725,13 +762,13 @@ impl TryFrom<&[u8]> for Integer { suffix: Some(suffix), }) } else { - Err(()) + Err(IntegerError::InvalidFormat) } } } impl TryFrom> for Integer { - type Error = (); + type Error = IntegerError; #[inline] fn try_from(value: Vec) -> Result { @@ -740,13 +777,13 @@ impl TryFrom> for Integer { } impl TryFrom> for Integer { - type Error = (); + type Error = IntegerError; #[inline] fn try_from(c: Cow<'_, [u8]>) -> Result { match c { Cow::Borrowed(c) => Self::try_from(c), - Cow::Owned(c) => Self::try_from(c).map_err(|_| ()), + Cow::Owned(c) => Self::try_from(c), } } } @@ -815,7 +852,7 @@ impl Serialize for IntegerSuffix { } impl FromStr for IntegerSuffix { - type Err = (); + type Err = IntegerError; #[inline] fn from_str(s: &str) -> Result { @@ -823,22 +860,22 @@ impl FromStr for IntegerSuffix { "k" => Ok(Self::Kibi), "m" => Ok(Self::Mebi), "g" => Ok(Self::Gibi), - _ => Err(()), + _ => Err(IntegerError::InvalidSuffix), } } } impl TryFrom<&[u8]> for IntegerSuffix { - type Error = (); + type Error = IntegerError; #[inline] fn try_from(s: &[u8]) -> Result { - Self::from_str(std::str::from_utf8(s).map_err(|_| ())?).map_err(|_| ()) + Self::from_str(std::str::from_utf8(s)?) } } impl TryFrom> for IntegerSuffix { - type Error = (); + type Error = IntegerError; #[inline] fn try_from(value: Vec) -> Result { @@ -903,12 +940,31 @@ impl Serialize for Color { } } +quick_error! { + #[derive(Debug, PartialEq)] + /// + #[allow(missing_docs)] + pub enum ColorError { + Utf8Conversion(err: std::str::Utf8Error) { + display("Ill-formed UTF-8") + source(err) + from() + } + InvalidColorItem { + display("Invalid color item") + } + InvalidFormat { + display("Invalid argument format") + } + } +} + impl TryFrom<&[u8]> for Color { - type Error = (); + type Error = ColorError; #[inline] fn try_from(s: &[u8]) -> Result { - let s = std::str::from_utf8(s).map_err(|_| ())?; + let s = std::str::from_utf8(s)?; enum ColorItem { Value(ColorValue), Attr(ColorAttribute), @@ -936,12 +992,12 @@ impl TryFrom<&[u8]> for Color { } else if new_self.background.is_none() { new_self.background = Some(v); } else { - return Err(()); + return Err(ColorError::InvalidColorItem); } } ColorItem::Attr(a) => new_self.attributes.push(a), }, - Err(_) => return Err(()), + Err(_) => return Err(ColorError::InvalidColorItem), } } @@ -950,7 +1006,7 @@ impl TryFrom<&[u8]> for Color { } impl TryFrom> for Color { - type Error = (); + type Error = ColorError; #[inline] fn try_from(value: Vec) -> Result { @@ -959,13 +1015,13 @@ impl TryFrom> for Color { } impl TryFrom> for Color { - type Error = (); + type Error = ColorError; #[inline] fn try_from(c: Cow<'_, [u8]>) -> Result { match c { Cow::Borrowed(c) => Self::try_from(c), - Cow::Owned(c) => Self::try_from(c).map_err(|_| ()), + Cow::Owned(c) => Self::try_from(c), } } } @@ -1049,7 +1105,7 @@ impl Serialize for ColorValue { } impl FromStr for ColorValue { - type Err = (); + type Err = ColorError; fn from_str(s: &str) -> Result { let mut s = s; @@ -1062,7 +1118,7 @@ impl FromStr for ColorValue { match s { "normal" if !bright => return Ok(Self::Normal), - "normal" if bright => return Err(()), + "normal" if bright => return Err(ColorError::InvalidFormat), "black" if !bright => return Ok(Self::Black), "black" if bright => return Ok(Self::BrightBlack), "red" if !bright => return Ok(Self::Red), @@ -1100,16 +1156,16 @@ impl FromStr for ColorValue { } } - Err(()) + Err(ColorError::InvalidFormat) } } impl TryFrom<&[u8]> for ColorValue { - type Error = (); + type Error = ColorError; #[inline] fn try_from(s: &[u8]) -> Result { - Self::from_str(std::str::from_utf8(s).map_err(|_| ())?).map_err(|_| ()) + Self::from_str(std::str::from_utf8(s)?) } } @@ -1184,7 +1240,7 @@ impl Serialize for ColorAttribute { } impl FromStr for ColorAttribute { - type Err = (); + type Err = ColorError; fn from_str(s: &str) -> Result { let inverted = s.starts_with("no"); @@ -1213,17 +1269,17 @@ impl FromStr for ColorAttribute { "italic" if inverted => Ok(Self::NoItalic), "strike" if !inverted => Ok(Self::Strike), "strike" if inverted => Ok(Self::NoStrike), - _ => Err(()), + _ => Err(ColorError::InvalidFormat), } } } impl TryFrom<&[u8]> for ColorAttribute { - type Error = (); + type Error = ColorError; #[inline] fn try_from(s: &[u8]) -> Result { - Self::from_str(std::str::from_utf8(s).map_err(|_| ())?).map_err(|_| ()) + Self::from_str(std::str::from_utf8(s)?) } } diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 410f54f2d14..7bd2eb60ef6 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -73,8 +73,8 @@ fn get_value_for_all_provided_values() -> crate::Result { ); assert_eq!( - file.value::("core", None, "other")?, - Value { + file.value::("core", None, "other")?, + Bytes { value: Cow::Borrowed(b"hello world") } ); From aaeec8b8ce620fa4e72b2a69a4ec515f9b502fa3 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Tue, 22 Feb 2022 22:33:37 +0100 Subject: [PATCH 09/11] Rename `Bytes` to `String`. --- git-config/src/file/git_config.rs | 16 ++++++++-------- git-config/src/values.rs | 12 ++++++------ .../integration_tests/file_integeration_test.rs | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index f6b2cd38f83..1f6df6c3d50 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -326,7 +326,7 @@ impl<'event> GitConfig<'event> { /// /// ``` /// # use git_config::file::{GitConfig, GitConfigError}; - /// # use git_config::values::{Integer, Bytes, Boolean, TrueVariant}; + /// # use git_config::values::{Integer, String, Boolean, TrueVariant}; /// # use std::borrow::Cow; /// # use std::convert::TryFrom; /// let config = r#" @@ -349,8 +349,8 @@ impl<'event> GitConfig<'event> { /// ] /// ); /// // ... or explicitly declare the type to avoid the turbofish - /// let c_value: Vec = git_config.multi_value("core", None, "c")?; - /// assert_eq!(c_value, vec![Bytes { value: Cow::Borrowed(b"g") }]); + /// let c_value: Vec = git_config.multi_value("core", None, "c")?; + /// assert_eq!(c_value, vec![String { value: Cow::Borrowed(b"g") }]); /// # Ok::<(), GitConfigError>(()) /// ``` /// @@ -2094,17 +2094,17 @@ mod get_value { use std::error::Error; use super::{Cow, GitConfig, TryFrom}; - use crate::values::{Boolean, Bytes, TrueVariant}; + use crate::values::{Boolean, String, TrueVariant}; #[test] fn single_section() -> Result<(), Box> { let config = GitConfig::try_from("[core]\na=b\nc").unwrap(); - let first_value: Bytes = config.value("core", None, "a")?; + let first_value: String = config.value("core", None, "a")?; let second_value: Boolean = config.value("core", None, "c")?; assert_eq!( first_value, - Bytes { + String { value: Cow::Borrowed(b"b") } ); @@ -2127,10 +2127,10 @@ mod get_value { "#; let config = GitConfig::try_from(config).unwrap(); - let value = config.value::("remote", Some("origin"), "url").unwrap(); + let value = config.value::("remote", Some("origin"), "url").unwrap(); assert_eq!( value, - Bytes { + String { value: Cow::Borrowed(b"git@github.com:Byron/gitoxide.git") } ); diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 66a843eef57..6c9418e60ad 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -146,12 +146,12 @@ fn b(s: &str) -> &[u8] { /// Any string value #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -pub struct Bytes<'a> { +pub struct String<'a> { /// bytes pub value: Cow<'a, [u8]>, } -impl<'a> From<&'a [u8]> for Bytes<'a> { +impl<'a> From<&'a [u8]> for String<'a> { #[inline] fn from(s: &'a [u8]) -> Self { Self { @@ -160,13 +160,13 @@ impl<'a> From<&'a [u8]> for Bytes<'a> { } } -impl From> for Bytes<'_> { +impl From> for String<'_> { fn from(s: Vec) -> Self { Self { value: Cow::Owned(s) } } } -impl<'a> From> for Bytes<'a> { +impl<'a> From> for String<'a> { #[inline] fn from(c: Cow<'a, [u8]>) -> Self { match c { @@ -512,7 +512,7 @@ impl TryFrom> for Boolean<'_> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(Cow::Owned(String::from_utf8(value)?))); + return Ok(Self::False(Cow::Owned(std::string::String::from_utf8(value)?))); } TrueVariant::try_from(value).map(Self::True) @@ -624,7 +624,7 @@ impl TryFrom> for TrueVariant<'_> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(Cow::Owned(String::from_utf8(value)?))) + Ok(Self::Explicit(Cow::Owned(std::string::String::from_utf8(value)?))) } else if value.is_empty() { Ok(Self::Implicit) } else { diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index 7bd2eb60ef6..a29b9746301 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -73,8 +73,8 @@ fn get_value_for_all_provided_values() -> crate::Result { ); assert_eq!( - file.value::("core", None, "other")?, - Bytes { + file.value::("core", None, "other")?, + String { value: Cow::Borrowed(b"hello world") } ); From acde3315d7960ee0f1145cc482305f77685d2c59 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Tue, 22 Feb 2022 22:33:37 +0100 Subject: [PATCH 10/11] Rename `Bytes` to `String`. --- git-config/src/values.rs | 43 ++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/git-config/src/values.rs b/git-config/src/values.rs index 6c9418e60ad..0bf9429c698 100644 --- a/git-config/src/values.rs +++ b/git-config/src/values.rs @@ -140,18 +140,19 @@ pub fn normalize_str(input: &str) -> Cow<'_, [u8]> { } /// Converts string to byte slice +#[cfg(test)] fn b(s: &str) -> &[u8] { s.as_bytes() } /// Any string value #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)] -pub struct String<'a> { +pub struct Bytes<'a> { /// bytes pub value: Cow<'a, [u8]>, } -impl<'a> From<&'a [u8]> for String<'a> { +impl<'a> From<&'a [u8]> for Bytes<'a> { #[inline] fn from(s: &'a [u8]) -> Self { Self { @@ -160,13 +161,13 @@ impl<'a> From<&'a [u8]> for String<'a> { } } -impl From> for String<'_> { +impl From> for Bytes<'_> { fn from(s: Vec) -> Self { Self { value: Cow::Owned(s) } } } -impl<'a> From> for String<'a> { +impl<'a> From> for Bytes<'a> { #[inline] fn from(c: Cow<'a, [u8]>) -> Self { match c { @@ -176,18 +177,6 @@ impl<'a> From> for String<'a> { } } -#[cfg(feature = "serde")] -impl Serialize for Value<'_> { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - match self { - Value(i) => i.serialize(serializer), - } - } -} - /// pub mod path { use std::borrow::Cow; @@ -469,12 +458,6 @@ quick_error! { /// The error returned when creating `Boolean` from byte string. #[allow(missing_docs)] pub enum BooleanError { - Utf8Conversion(err: std::str::Utf8Error) { - display("Ill-formed UTF-8") - source(err) - from() - from(err: std::string::FromUtf8Error) -> (err.utf8_error()) - } InvalidFormat { display("Invalid argument format") } @@ -495,7 +478,9 @@ impl<'a> TryFrom<&'a [u8]> for Boolean<'a> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(std::str::from_utf8(value)?.into())); + return Ok(Self::False( + std::str::from_utf8(value).expect("value is already validated").into(), + )); } Err(BooleanError::InvalidFormat) @@ -512,7 +497,9 @@ impl TryFrom> for Boolean<'_> { || value.eq_ignore_ascii_case(b"zero") || value == b"\"\"" { - return Ok(Self::False(Cow::Owned(std::string::String::from_utf8(value)?))); + return Ok(Self::False(Cow::Owned( + std::string::String::from_utf8(value).expect("value is already validated"), + ))); } TrueVariant::try_from(value).map(Self::True) @@ -606,7 +593,9 @@ impl<'a> TryFrom<&'a [u8]> for TrueVariant<'a> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(std::str::from_utf8(value)?.into())) + Ok(Self::Explicit( + std::str::from_utf8(value).expect("value is already validated").into(), + )) } else if value.is_empty() { Ok(Self::Implicit) } else { @@ -624,7 +613,9 @@ impl TryFrom> for TrueVariant<'_> { || value.eq_ignore_ascii_case(b"true") || value.eq_ignore_ascii_case(b"one") { - Ok(Self::Explicit(Cow::Owned(std::string::String::from_utf8(value)?))) + Ok(Self::Explicit(Cow::Owned( + std::string::String::from_utf8(value).expect("value is already validated"), + ))) } else if value.is_empty() { Ok(Self::Implicit) } else { From cabe071dbf251db5e4f7740ca4a9aac4201bcb32 Mon Sep 17 00:00:00 2001 From: Svetlin Stefanov Date: Sat, 5 Mar 2022 11:00:58 +0100 Subject: [PATCH 11/11] Rename `String` to `Bytes`. --- git-config/src/file/git_config.rs | 16 ++++++++-------- .../integration_tests/file_integeration_test.rs | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/git-config/src/file/git_config.rs b/git-config/src/file/git_config.rs index 1f6df6c3d50..f6b2cd38f83 100644 --- a/git-config/src/file/git_config.rs +++ b/git-config/src/file/git_config.rs @@ -326,7 +326,7 @@ impl<'event> GitConfig<'event> { /// /// ``` /// # use git_config::file::{GitConfig, GitConfigError}; - /// # use git_config::values::{Integer, String, Boolean, TrueVariant}; + /// # use git_config::values::{Integer, Bytes, Boolean, TrueVariant}; /// # use std::borrow::Cow; /// # use std::convert::TryFrom; /// let config = r#" @@ -349,8 +349,8 @@ impl<'event> GitConfig<'event> { /// ] /// ); /// // ... or explicitly declare the type to avoid the turbofish - /// let c_value: Vec = git_config.multi_value("core", None, "c")?; - /// assert_eq!(c_value, vec![String { value: Cow::Borrowed(b"g") }]); + /// let c_value: Vec = git_config.multi_value("core", None, "c")?; + /// assert_eq!(c_value, vec![Bytes { value: Cow::Borrowed(b"g") }]); /// # Ok::<(), GitConfigError>(()) /// ``` /// @@ -2094,17 +2094,17 @@ mod get_value { use std::error::Error; use super::{Cow, GitConfig, TryFrom}; - use crate::values::{Boolean, String, TrueVariant}; + use crate::values::{Boolean, Bytes, TrueVariant}; #[test] fn single_section() -> Result<(), Box> { let config = GitConfig::try_from("[core]\na=b\nc").unwrap(); - let first_value: String = config.value("core", None, "a")?; + let first_value: Bytes = config.value("core", None, "a")?; let second_value: Boolean = config.value("core", None, "c")?; assert_eq!( first_value, - String { + Bytes { value: Cow::Borrowed(b"b") } ); @@ -2127,10 +2127,10 @@ mod get_value { "#; let config = GitConfig::try_from(config).unwrap(); - let value = config.value::("remote", Some("origin"), "url").unwrap(); + let value = config.value::("remote", Some("origin"), "url").unwrap(); assert_eq!( value, - String { + Bytes { value: Cow::Borrowed(b"git@github.com:Byron/gitoxide.git") } ); diff --git a/git-config/tests/integration_tests/file_integeration_test.rs b/git-config/tests/integration_tests/file_integeration_test.rs index a29b9746301..7bd2eb60ef6 100644 --- a/git-config/tests/integration_tests/file_integeration_test.rs +++ b/git-config/tests/integration_tests/file_integeration_test.rs @@ -73,8 +73,8 @@ fn get_value_for_all_provided_values() -> crate::Result { ); assert_eq!( - file.value::("core", None, "other")?, - String { + file.value::("core", None, "other")?, + Bytes { value: Cow::Borrowed(b"hello world") } );