fix!: rename WithSidebands::read_line() to *::read_line_to_string().

Also, we remove the existing blocking `read_line()` override as it's not
representable in the async implementation anyway, to unify both implementations
once again.

The new name also makes sure that we are not accidentally calling a built-in
implementation on the `BufRead` trait as it's clear our method has to be called
under a different name.

Author: Byron

gix-packetline/src/read/sidebands/async_io.rs

@@ -170,7 +170,7 @@ where
     }
 
     /// Read a packet line as string line.
-    pub fn read_line<'b>(&'b mut self, buf: &'b mut String) -> ReadLineFuture<'a, 'b, T, F> {
+    pub fn read_line_to_string<'b>(&'b mut self, buf: &'b mut String) -> ReadLineFuture<'a, 'b, T, F> {
         ReadLineFuture { parent: self, buf }
     }
 

gix-packetline/src/read/sidebands/blocking_io.rs

@@ -109,6 +109,22 @@ where
         );
         self.parent.read_line()
     }
+
+    /// Like `BufRead::read_line()`, but will only read one packetline at a time.
+    ///
+    /// It will also be easier to call as sometimes it's unclear which implementation we get on a type like this with
+    /// plenty of generic parameters.
+    pub fn read_line_to_string(&mut self, buf: &mut String) -> io::Result<usize> {
+        assert_eq!(
+            self.cap, 0,
+            "we don't support partial buffers right now - read-line must be used consistently"
+        );
+        let line = std::str::from_utf8(self.fill_buf()?).map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
+        buf.push_str(line);
+        let bytes = line.len();
+        self.cap = 0;
+        Ok(bytes)
+    }
 }
 
 impl<'a, T, F> BufRead for WithSidebands<'a, T, F>
@@ -168,18 +184,6 @@ where
     fn consume(&mut self, amt: usize) {
         self.pos = std::cmp::min(self.pos + amt, self.cap);
     }
-
-    fn read_line(&mut self, buf: &mut String) -> io::Result<usize> {
-        assert_eq!(
-            self.cap, 0,
-            "we don't support partial buffers right now - read-line must be used consistently"
-        );
-        let line = std::str::from_utf8(self.fill_buf()?).map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
-        buf.push_str(line);
-        let bytes = line.len();
-        self.cap = 0;
-        Ok(bytes)
-    }
 }
 
 impl<'a, T, F> io::Read for WithSidebands<'a, T, F>

gix-packetline/tests/read/sideband.rs

@@ -1,5 +1,5 @@
 #[cfg(feature = "blocking-io")]
-use std::io::{BufRead, Read};
+use std::io::Read;
 
 use bstr::{BString, ByteSlice};
 #[cfg(all(not(feature = "blocking-io"), feature = "async-io"))]
@@ -106,16 +106,16 @@ async fn read_line_trait_method_reads_one_packet_line_at_a_time() -> crate::Resu
 
     let mut out = String::new();
     let mut r = rd.as_read();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "808e50d724f604f69ab93c6da2919c014667bedb HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow deepen-since deepen-not deepen-relative no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master object-format=sha1 agent=git/2.28.0\n");
     out.clear();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "808e50d724f604f69ab93c6da2919c014667bedb refs/heads/master\n");
     out.clear();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "", "flush means empty lines…");
     out.clear();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "", "…which can't be overcome unless the reader is reset");
     assert_eq!(
         r.stopped_at(),
@@ -127,18 +127,18 @@ async fn read_line_trait_method_reads_one_packet_line_at_a_time() -> crate::Resu
     rd.reset();
 
     let mut r = rd.as_read();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "NAK\n");
 
     drop(r);
 
     let mut r = rd.as_read_with_sidebands(|_, _| ());
     out.clear();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "&");
 
     out.clear();
-    r.read_line(&mut out).await?;
+    r.read_line_to_string(&mut out).await?;
     assert_eq!(out, "");
 
     Ok(())
@@ -199,7 +199,7 @@ async fn peek_past_an_actual_eof_is_an_error() -> crate::Result {
     assert_eq!(res.expect("one line")??, b"ERR e");
 
     let mut buf = String::new();
-    reader.read_line(&mut buf).await?;
+    reader.read_line_to_string(&mut buf).await?;
     assert_eq!(
         buf, "ERR e",
         "by default ERR lines won't propagate as failure but are merely text"
@@ -223,7 +223,7 @@ async fn peek_past_a_delimiter_is_no_error() -> crate::Result {
     assert_eq!(res.expect("one line")??, b"hello");
 
     let mut buf = String::new();
-    reader.read_line(&mut buf).await?;
+    reader.read_line_to_string(&mut buf).await?;
     assert_eq!(buf, "hello");
 
     let res = reader.peek_data_line().await;