TryWriteable should have try_writeable_borrow
https://github.com/unicode-org/icu4x/pull/6985#discussion_r2389281839
No I don't think so. The borrowing method should be constant time, because otherwise write_to_string would duplicate work. I don't think a scenario where an error can be detected in constant time needs to be a TryWriteable in the first place, so there's no point in making the borrowing fallible.
ok, maybe it should return (Option<&str>, Option<&str>) then?
Or maybe TryWriteable just shouldn't have this function?
what would that pair mean?
Just the result of running writeable_borrow on both the Ok and the Err variants. If either needs to be computed, then it is None. For example, often TryWriteable is implement on Result<W, &'static str>, and in that case, try_writeable_borrow might return (None, Some(<the err string>)). Haven't fully thought it through.
How would try_write_to_string then know which variant to choose?
This method is required to make TryWriteable borrow in no-alloc mode, so I'd rather not remove it.
What if we had a signature like
trait NewWriteSink<'a>: fmt::Write {
fn write_finish(s: &'a str) -> fmt::Result;
}
trait TryWriteable {
// ...
fn try_write_bikeshed<'a, NewWriteSink>(&'a self, sink: NewWriteSink<'a>) -> Result<Result<(), Self::Error>, fmt::Error>;
}
// No-alloc impl:
struct NoAllocNewWriteSink<'a>(Option<&'a str>);
impl fmt::Write for NoAllocNewWriteSink<'_> {
// functions that set the inner option to None and return an error
}
impl<'a> NewWriteSink<'a> for NoAllocNewWriteSink<'a> {
fn write_finish(s: &'a str) -> fmt::Result {
self.0 = Some(s);
Ok(())
}
}
- @robertbastian We could just remove it in 2.1; we don't seem to use it
- @robertbastian We could use it in impl TryWriteable for PotentialUtf8
- @sffc That's a linear-time impl; is that ok?
- @robertbastian Yeah it could be ok
- @sffc I think we should be able to borrow in either the Ok or the Err case
- @sffc I think the correct signature is
Option<Result<&str, &str>> - @robertbastian Maybe, but it still needs the error code in the error case. So it would be
Option<Result<&str, (Self::Error, &str)>>. Let's just wait until we really need it
This can be removed from the milestone or closed, yes?