refactor

- cargo fmt
- remove unnecessary tests
- keep everything open

Author: Byron

gix-config-value/Cargo.toml

@@ -12,7 +12,7 @@ rust-version = "1.82"
 include = ["src/**/*", "LICENSE-*"]
 
 [lib]
-doctest = false
+doctest = true
 
 [features]
 ## Data structures implement `serde::Serialize` and `serde::Deserialize`.

gix-config-value/src/path.rs

@@ -4,9 +4,6 @@ use bstr::BStr;
 
 use crate::Path;
 
-/// The prefix used to mark a path as optional in Git configuration files.
-const OPTIONAL_PREFIX: &[u8] = b":(optional)";
-
 ///
 pub mod interpolate {
     use std::path::PathBuf;
@@ -111,7 +108,9 @@ impl AsRef<BStr> for Path<'_> {
 
 impl<'a> From<Cow<'a, BStr>> for Path<'a> {
     fn from(value: Cow<'a, BStr>) -> Self {
-        // Check if the value starts with ":(optional)" prefix
+        /// The prefix used to mark a path as optional in Git configuration files.
+        const OPTIONAL_PREFIX: &[u8] = b":(optional)";
+
         if value.starts_with(OPTIONAL_PREFIX) {
             // Strip the prefix while preserving the Cow variant for efficiency:
             // - Borrowed data remains borrowed (no allocation)
@@ -125,27 +124,18 @@ impl<'a> From<Cow<'a, BStr>> for Path<'a> {
             };
             Path {
                 value: stripped,
-                optional: true,
+                is_optional: true,
             }
         } else {
             Path {
                 value,
-                optional: false,
+                is_optional: false,
             }
         }
     }
 }
 
 impl<'a> Path<'a> {
-    /// Returns `true` if this path was prefixed with `:(optional)`.
-    ///
-    /// Optional paths indicate that it's acceptable if the file doesn't exist.
-    /// This is typically used for configuration like `blame.ignorerevsfile` where
-    /// the file might not exist in all repositories.
-    pub fn is_optional(&self) -> bool {
-        self.optional
-    }
-
     /// Interpolates this path into a path usable on the file system.
     ///
     /// If this path starts with `~/` or `~user/` or `%(prefix)/`
@@ -158,8 +148,8 @@ impl<'a> Path<'a> {
     ///    This location is not known at compile time and therefore need to be
     ///    optionally provided by the caller through `git_install_dir`.
     ///
-    /// Any other, non-empty path value is returned unchanged and error is returned in case of an empty path value or if required input
-    /// wasn't provided.
+    /// Any other, non-empty path value is returned unchanged and error is returned in case of an empty path value or if the required
+    /// input wasn't provided.
     pub fn interpolate(
         self,
         interpolate::Context {

gix-config-value/src/types.rs

@@ -46,7 +46,7 @@ pub struct Boolean(pub bool);
 ///
 /// Paths can be marked as optional by prefixing them with `:(optional)` in the configuration.
 /// This indicates that it's acceptable if the file doesn't exist, which is useful for
-/// configuration values like `blame.ignorerevsfile` that may only exist in some repositories.
+/// configuration values like `blame.ignoreRevsFile` that may only exist in some repositories.
 ///
 /// ```
 /// use std::borrow::Cow;
@@ -55,18 +55,21 @@ pub struct Boolean(pub bool);
 ///
 /// // Regular path - file is expected to exist
 /// let path = Path::from(Cow::Borrowed(b"/etc/gitconfig".as_bstr()));
-/// assert!(!path.is_optional());
+/// assert!(!path.is_optional);
 ///
 /// // Optional path - it's okay if the file doesn't exist
 /// let path = Path::from(Cow::Borrowed(b":(optional)~/.gitignore".as_bstr()));
-/// assert!(path.is_optional());
+/// assert!(path.is_optional);
 /// assert_eq!(path.value.as_ref(), b"~/.gitignore"); // prefix is stripped
 /// ```
 #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]
 pub struct Path<'a> {
     /// The path string, un-interpolated
     pub value: std::borrow::Cow<'a, bstr::BStr>,
     /// Whether this path was prefixed with `:(optional)`, indicating it's acceptable if the file doesn't exist.
-    /// Use `is_optional()` method to check this value.
-    pub(crate) optional: bool,
+    ///
+    /// Optional paths indicate that it's acceptable if the file doesn't exist.
+    /// This is typically used for configuration like `blame.ignorerevsfile` where
+    /// the file might not exist in all repositories.
+    pub is_optional: bool,
 }

gix-config-value/tests/value/main.rs

@@ -15,23 +15,3 @@ mod boolean;
 mod color;
 mod integer;
 mod path;
-
-/// Ensure that the `:(optional)` prefix is only recognized for Path types, not for other types
-mod optional_prefix_only_for_paths {
-    use std::borrow::Cow;
-    use gix_config_value::{Boolean, Integer};
-
-    #[test]
-    fn optional_prefix_not_recognized_in_boolean() {
-        // Boolean should fail to parse this because it's not a valid boolean value
-        let result = Boolean::try_from(Cow::Borrowed(crate::b(":(optional)true")));
-        assert!(result.is_err(), "Boolean should not recognize :(optional) prefix");
-    }
-
-    #[test]
-    fn optional_prefix_not_recognized_in_integer() {
-        // Integer should fail to parse this because it's not a valid integer value
-        let result = Integer::try_from(Cow::Borrowed(crate::b(":(optional)42")));
-        assert!(result.is_err(), "Integer should not recognize :(optional) prefix");
-    }
-}

gix-config-value/tests/value/path.rs

@@ -131,111 +131,113 @@ mod interpolate {
 mod optional_prefix {
     use std::borrow::Cow;
 
-    use bstr::ByteSlice;
     use crate::{b, cow_str};
+    use bstr::ByteSlice;
 
     #[test]
     fn path_without_optional_prefix_is_not_optional() {
         let path = gix_config_value::Path::from(Cow::Borrowed(b("/some/path")));
-        assert!(!path.is_optional(), "path without prefix should not be optional");
+        assert!(!path.is_optional, "path without prefix should not be optional");
         assert_eq!(path.value.as_ref(), b"/some/path");
     }
 
     #[test]
     fn path_with_optional_prefix_is_optional() {
         let path = gix_config_value::Path::from(cow_str(":(optional)/some/path"));
-        assert!(path.is_optional(), "path with :(optional) prefix should be optional");
+        assert!(path.is_optional, "path with :(optional) prefix should be optional");
         assert_eq!(path.value.as_ref(), b"/some/path", "prefix should be stripped");
     }
 
     #[test]
     fn optional_prefix_with_relative_path() {
         let path = gix_config_value::Path::from(cow_str(":(optional)relative/path"));
-        assert!(path.is_optional());
+        assert!(path.is_optional);
         assert_eq!(path.value.as_ref(), b"relative/path");
     }
 
     #[test]
     fn optional_prefix_with_tilde_expansion() {
         let path = gix_config_value::Path::from(cow_str(":(optional)~/config/file"));
-        assert!(path.is_optional());
-        assert_eq!(path.value.as_ref(), b"~/config/file", "tilde should be preserved for interpolation");
+        assert!(path.is_optional);
+        assert_eq!(
+            path.value.as_ref(),
+            b"~/config/file",
+            "tilde should be preserved for interpolation"
+        );
     }
 
     #[test]
     fn optional_prefix_with_prefix_substitution() {
         let path = gix_config_value::Path::from(cow_str(":(optional)%(prefix)/share/git"));
-        assert!(path.is_optional());
-        assert_eq!(path.value.as_ref(), b"%(prefix)/share/git", "prefix should be preserved for interpolation");
+        assert!(path.is_optional);
+        assert_eq!(
+            path.value.as_ref(),
+            b"%(prefix)/share/git",
+            "prefix should be preserved for interpolation"
+        );
     }
 
     #[test]
     fn optional_prefix_with_windows_path() {
         let path = gix_config_value::Path::from(cow_str(r":(optional)C:\Users\file"));
-        assert!(path.is_optional());
+        assert!(path.is_optional);
         assert_eq!(path.value.as_ref(), br"C:\Users\file");
     }
 
     #[test]
     fn optional_prefix_followed_by_empty_path() {
         let path = gix_config_value::Path::from(cow_str(":(optional)"));
-        assert!(path.is_optional());
+        assert!(path.is_optional);
         assert_eq!(path.value.as_ref(), b"", "empty path after prefix is valid");
     }
 
     #[test]
     fn partial_optional_string_is_not_treated_as_prefix() {
         let path = gix_config_value::Path::from(cow_str(":(opt)ional/path"));
-        assert!(!path.is_optional(), "incomplete prefix should not be treated as optional marker");
+        assert!(
+            !path.is_optional,
+            "incomplete prefix should not be treated as optional marker"
+        );
         assert_eq!(path.value.as_ref(), b":(opt)ional/path");
     }
 
     #[test]
     fn optional_prefix_case_sensitive() {
         let path = gix_config_value::Path::from(cow_str(":(OPTIONAL)/some/path"));
-        assert!(!path.is_optional(), "prefix should be case-sensitive");
+        assert!(!path.is_optional, "prefix should be case-sensitive");
         assert_eq!(path.value.as_ref(), b":(OPTIONAL)/some/path");
     }
 
     #[test]
     fn optional_prefix_with_spaces() {
         let path = gix_config_value::Path::from(cow_str(":(optional) /path/with/space"));
-        assert!(path.is_optional());
-        assert_eq!(path.value.as_ref(), b" /path/with/space", "space after prefix should be preserved");
-    }
-
-    #[test]
-    fn interpolate_preserves_optional_flag() -> crate::Result {
-        use gix_config_value::path;
-        
-        let path = gix_config_value::Path::from(cow_str(":(optional)/absolute/path"));
-        assert!(path.is_optional());
-        
-        let interpolated = path.interpolate(path::interpolate::Context::default())?;
-        assert_eq!(interpolated.as_ref(), std::path::Path::new("/absolute/path"));
-        
-        Ok(())
+        assert!(path.is_optional);
+        assert_eq!(
+            path.value.as_ref(),
+            b" /path/with/space",
+            "space after prefix should be preserved"
+        );
     }
 
     #[test]
     fn borrowed_path_stays_borrowed_after_prefix_stripping() {
         // Verify that we don't unnecessarily allocate when stripping the prefix from borrowed data
         let borrowed_input: &[u8] = b":(optional)/some/path";
         let path = gix_config_value::Path::from(Cow::Borrowed(borrowed_input.as_bstr()));
-        
-        assert!(path.is_optional());
+
+        assert!(path.is_optional);
         assert_eq!(path.value.as_ref(), b"/some/path");
         // Verify it's still borrowed (no unnecessary allocation)
         assert!(matches!(path.value, Cow::Borrowed(_)));
     }
 
     #[test]
     fn owned_path_stays_owned_after_prefix_stripping() {
-        // Verify that owned data remains owned after prefix stripping (but efficiently)
+        // Verify that owned data remains owned after prefix stripping
         let owned_input = bstr::BString::from(":(optional)/some/path");
         let path = gix_config_value::Path::from(Cow::Owned(owned_input));
-        
-        assert!(path.is_optional());
+
+        assert!(path.is_optional);
         assert_eq!(path.value.as_ref(), b"/some/path");
         // Verify it's still owned
         assert!(matches!(path.value, Cow::Owned(_)));