[197g]

Sketched with the interface as proposed here: #97 (comment) except that I made free() a Result<Vec<_>, Fail>. Since the argument processing would usually return an error before in the constructor and we no longer have that verification, a panic might be surprising to callers that had previously relied on that.

In my opinion the necessary doctest changes suggest this is not super ergonomic yet. I've not yet added a lot of documentation to the methods in case the interface still needs changes.

Additionally it seems odd to treat Val and free arguments differently but that might be a tradeoff worth making. Your call.

[tgross35]

Thanks for looking at this!

In my opinion the necessary doctest changes suggest this is not super ergonomic yet. I've not yet added a lot of documentation to the methods in case the interface still needs changes.

The doctests can use .free() rather than .free_os() right? That's what we will probably expect most users to do, free_os only exists in case you actually expect to be handling non-UTF-8 strings.

[tgross35]

As part of that, I think .free() should panic and suggest using free_os() rather than returning a Result.

(I'm not 100% convinced of this yet, it's just more ergonomic)

[197g]

It's not really the OsString which is awkward to handle. That part seemed fine and by wrapping the expected side in an OsStr::new constructor the comparison does not stand out much. What bothers me, at least now I'm able to put it into words better, is the way of accessing individual arguments if that were written with free:

    /// assert_eq!(matches.free()[end_pos], "arg2");
                           ^^^^^^^^^^^^^^^ indexing into temporary Vec

//!         matches.free()[0].clone()
                    ^^^^^^^^^^^^^^^^^ here too

It feels really dirty to do the full allocation when the documentation demonstrates mostly individual argument access. Maybe it should utilized an iterator instead (but when to panic then)?

[A4-Tacks]

I think it should be:

• fn free(&self) -> &[String] (uses similar to LazyLock<Vec<String>> logic)
• fn free_os(&self) -> &[OsString]

This is consistent, easy to use, and effective panic flow

[matthewhughes934]

I think it should be:

• fn free(&self) -> &[String] (uses similar to LazyLock<Vec<String>> logic)

• fn free_os(&self) -> &[OsString]

This is consistent, easy to use, and effective panic flow

What about tracking it with an Option<Vec<String>>, quick PoC:

diff --git i/src/lib.rs w/src/lib.rs
index 0fe9ba5..5ee550e 100644
--- i/src/lib.rs
+++ w/src/lib.rs
@@ -574,11 +574,15 @@ pub fn parse<C: IntoIterator>(&self, args: C) -> Result
         // in option does not exist in `free` and must be replaced with `None`
         args_end = args_end.filter(|pos| pos != &free.len());
 
+        let free_str: Option<Vec<String>> =
+            free.iter().map(|o| o.to_str().map(str::to_owned)).collect();
+
         Ok(Matches {
             opts,
             vals,
             free,
             args_end,
+            free_str,
         })
     }
 
@@ -804,6 +808,8 @@ pub struct Matches {
     free: Vec<OsString>,
     /// Index of first free fragment after "--" separator
     args_end: Option<usize>,
+
+    free_str: Option<Vec<String>>,
 }
 
 /// The type returned when the command line does not conform to the
@@ -1172,8 +1178,11 @@ pub fn free_trailing_start(&self) -> Option<usize> {
     }
 
     /// Returns a vector of free arguments if they are strings.
-    pub fn free(&self) -> result::Result<Vec<&str>, Fail> {
-        self.free.iter().map(|os| non_string_to_err(os)).collect()
+    pub fn free(&self) -> &[String] {
+        &self
+            .free_str
+            .as_ref()
+            .expect("free contained non-UTF-8 strings, use `free_os` to handle this")
     }
 
     /// Returns a slice to all free arguments.

[A4-Tacks]

Always parsing strings: additional costs and memory usage, imagine a list of files with a length of 100000
Lazy logic, if not used, there is no cost:

struct Matches {
    free: Vec<OsString>,
    free_str: OnceLock<Box<[String]>>,
}
impl Matches {
    fn free_os(&self) -> &[OsString] {
        &self.free
    }

    fn free(&self) -> &[String] {
        self.free_str.get_or_init(|| {
            self.free
                .iter()
                .map(|it| {
                    it.to_str()
                        .map(str::to_owned)
                        // It seems like we can't track_caller, maybe we need a Option<Box<[String]>>?
                        .expect("free contained non-UTF-8 strings, use `free_os` to handle this")
                })
                .collect()
        })
    }
}