Skip to content

Commit

Permalink
[packetline #178] refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 26, 2021
1 parent d4c16a9 commit 23438fd
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 92 deletions.
5 changes: 4 additions & 1 deletion git-packetline/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@

* **renames / moves**
- `immutable::PacketLine` -> `PacketLineRef`
- `immutable::Error` -> `ErrorRef`
- `immutable::Text` -> `TextRef`
- `immutable::Band` -> `BandRef`
- `immutable::DecodeBandError` -> `decode::band::Error`

- `pub immutable::` -> `line::`
25 changes: 24 additions & 1 deletion git-packetline/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ pub enum Channel {
}

///
pub mod immutable;
pub mod line;

/// A borrowed packet line as it refers to a slice of data by reference.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
Expand All @@ -39,6 +40,28 @@ pub enum PacketLineRef<'a> {
ResponseEnd,
}

/// A packet line representing an Error in a side-band channel.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
pub struct ErrorRef<'a>(pub &'a [u8]);

/// A packet line representing text, which may include a trailing newline.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
pub struct TextRef<'a>(pub &'a [u8]);

/// A band in a side-band channel.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
pub enum BandRef<'a> {
/// A band carrying data.
Data(&'a [u8]),
/// A band carrying user readable progress information.
Progress(&'a [u8]),
/// A band carrying user readable errors.
Error(&'a [u8]),
}

///
pub mod read;
#[doc(inline)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,30 @@ use std::io;

use futures_io::AsyncWrite;

use crate::{
encode,
immutable::{Band, Error, Text},
Channel, PacketLineRef,
};
use crate::{encode, BandRef, Channel, ErrorRef, PacketLineRef, TextRef};

impl<'a> Band<'a> {
impl<'a> BandRef<'a> {
/// Serialize this instance to `out`, returning the amount of bytes written.
///
/// The data written to `out` can be decoded with [`Borrowed::decode_band()]`.
pub async fn write_to(&self, out: impl AsyncWrite + Unpin) -> io::Result<usize> {
match self {
Band::Data(d) => encode::band_to_write(Channel::Data, d, out),
Band::Progress(d) => encode::band_to_write(Channel::Progress, d, out),
Band::Error(d) => encode::band_to_write(Channel::Error, d, out),
BandRef::Data(d) => encode::band_to_write(Channel::Data, d, out),
BandRef::Progress(d) => encode::band_to_write(Channel::Progress, d, out),
BandRef::Error(d) => encode::band_to_write(Channel::Error, d, out),
}
.await
}
}

impl<'a> Text<'a> {
impl<'a> TextRef<'a> {
/// Serialize this instance to `out`, appending a newline if there is none, returning the amount of bytes written.
pub async fn write_to(&self, out: impl AsyncWrite + Unpin) -> io::Result<usize> {
encode::text_to_write(self.0, out).await
}
}

impl<'a> Error<'a> {
impl<'a> ErrorRef<'a> {
/// Serialize this line as error to `out`.
///
/// This includes a marker to allow decoding it outside of a side-band channel, returning the amount of bytes written.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,28 @@
use std::io;

use crate::{
encode,
immutable::{Band, Error, Text},
Channel, PacketLineRef,
};
use crate::{encode, BandRef, Channel, ErrorRef, PacketLineRef, TextRef};

impl<'a> Band<'a> {
impl<'a> BandRef<'a> {
/// Serialize this instance to `out`, returning the amount of bytes written.
///
/// The data written to `out` can be decoded with [`Borrowed::decode_band()]`.
pub fn write_to(&self, out: impl io::Write) -> io::Result<usize> {
match self {
Band::Data(d) => encode::band_to_write(Channel::Data, d, out),
Band::Progress(d) => encode::band_to_write(Channel::Progress, d, out),
Band::Error(d) => encode::band_to_write(Channel::Error, d, out),
BandRef::Data(d) => encode::band_to_write(Channel::Data, d, out),
BandRef::Progress(d) => encode::band_to_write(Channel::Progress, d, out),
BandRef::Error(d) => encode::band_to_write(Channel::Error, d, out),
}
}
}

impl<'a> Text<'a> {
impl<'a> TextRef<'a> {
/// Serialize this instance to `out`, appending a newline if there is none, returning the amount of bytes written.
pub fn write_to(&self, out: impl io::Write) -> io::Result<usize> {
encode::text_to_write(self.0, out)
}
}

impl<'a> Error<'a> {
impl<'a> ErrorRef<'a> {
/// Serialize this line as error to `out`.
///
/// This includes a marker to allow decoding it outside of a side-band channel, returning the amount of bytes written.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bstr::BStr;

use crate::{decode, Channel, PacketLineRef, ERR_PREFIX};
use crate::{decode, BandRef, Channel, ErrorRef, PacketLineRef, TextRef, ERR_PREFIX};

impl<'a> PacketLineRef<'a> {
/// Return this instance as slice if it's [`Data`][PacketLine::Data].
Expand All @@ -20,68 +20,58 @@ impl<'a> PacketLineRef<'a> {
///
/// Note that this creates an unchecked error using the slice verbatim, which is useful to [serialize it][Error::write_to()].
/// See [`check_error()`][PacketLine::check_error()] for a version that assures the error information is in the expected format.
pub fn as_error(&self) -> Option<Error<'_>> {
self.as_slice().map(Error)
pub fn as_error(&self) -> Option<ErrorRef<'_>> {
self.as_slice().map(ErrorRef)
}
/// Check this instance's [`as_slice()`][PacketLine::as_slice()] is a valid [`Error`] and return it.
///
/// This works for any data received in an error [channel][crate::Channel].
pub fn check_error(&self) -> Option<Error<'_>> {
pub fn check_error(&self) -> Option<ErrorRef<'_>> {
self.as_slice().and_then(|data| {
if data.len() >= ERR_PREFIX.len() && &data[..ERR_PREFIX.len()] == ERR_PREFIX {
Some(Error(&data[ERR_PREFIX.len()..]))
Some(ErrorRef(&data[ERR_PREFIX.len()..]))
} else {
None
}
})
}
/// Return this instance as text, with the trailing newline truncated if present.
pub fn as_text(&self) -> Option<Text<'_>> {
pub fn as_text(&self) -> Option<TextRef<'_>> {
self.as_slice().map(Into::into)
}

/// Interpret the data in this [`slice`][PacketLine::as_slice()] as [`Band`] according to the given `kind` of channel.
///
/// Note that this is only relevant in a side-band channel.
/// See [`decode_band()`][PacketLine::decode_band()] in case `kind` is unknown.
pub fn as_band(&self, kind: Channel) -> Option<Band<'_>> {
pub fn as_band(&self, kind: Channel) -> Option<BandRef<'_>> {
self.as_slice().map(|d| match kind {
Channel::Data => Band::Data(d),
Channel::Progress => Band::Progress(d),
Channel::Error => Band::Error(d),
Channel::Data => BandRef::Data(d),
Channel::Progress => BandRef::Progress(d),
Channel::Error => BandRef::Error(d),
})
}

/// Decode the band of this [`slice`][PacketLine::as_slice()], or panic if it is not actually a side-band line.
pub fn decode_band(&self) -> Result<Band<'_>, decode::band::Error> {
pub fn decode_band(&self) -> Result<BandRef<'_>, decode::band::Error> {
let d = self.as_slice().ok_or(decode::band::Error::NonDataLine)?;
Ok(match d[0] {
1 => Band::Data(&d[1..]),
2 => Band::Progress(&d[1..]),
3 => Band::Error(&d[1..]),
1 => BandRef::Data(&d[1..]),
2 => BandRef::Progress(&d[1..]),
3 => BandRef::Error(&d[1..]),
band => return Err(decode::band::Error::InvalidSideBand(band)),
})
}
}

/// A packet line representing an Error in a side-band channel.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
pub struct Error<'a>(pub &'a [u8]);

/// A packet line representing text, which may include a trailing newline.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
pub struct Text<'a>(pub &'a [u8]);

impl<'a> From<&'a [u8]> for Text<'a> {
impl<'a> From<&'a [u8]> for TextRef<'a> {
fn from(d: &'a [u8]) -> Self {
let d = if d[d.len() - 1] == b'\n' { &d[..d.len() - 1] } else { d };
Text(d)
TextRef(d)
}
}

impl<'a> Text<'a> {
impl<'a> TextRef<'a> {
/// Return this instance's data.
pub fn as_slice(&self) -> &[u8] {
self.0
Expand All @@ -92,18 +82,6 @@ impl<'a> Text<'a> {
}
}

/// A band in a side-band channel.
#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))]
pub enum Band<'a> {
/// A band carrying data.
Data(&'a [u8]),
/// A band carrying user readable progress information.
Progress(&'a [u8]),
/// A band carrying user readable errors.
Error(&'a [u8]),
}

#[cfg(all(not(feature = "blocking-io"), feature = "async-io"))]
mod async_io;
#[cfg(feature = "blocking-io")]
Expand Down
16 changes: 6 additions & 10 deletions git-packetline/src/read/sidebands/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ use std::{
use futures_io::{AsyncBufRead, AsyncRead};
use futures_lite::ready;

use crate::{
decode,
immutable::{Band, Text},
PacketLineRef, StreamingPeekableIter, U16_HEX_BYTES,
};
use crate::{decode, BandRef, PacketLineRef, StreamingPeekableIter, TextRef, U16_HEX_BYTES};

type ReadLineResult<'a> = Option<std::io::Result<Result<PacketLineRef<'a>, decode::Error>>>;
/// An implementor of [`AsyncBufRead`] yielding packet lines on each call to [`read_line()`][AsyncBufRead::read_line()].
Expand Down Expand Up @@ -238,13 +234,13 @@ where
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
const ENCODED_BAND: usize = 1;
match band {
Band::Data(d) => break (U16_HEX_BYTES + ENCODED_BAND, d.len()),
Band::Progress(d) => {
let text = Text::from(d).0;
BandRef::Data(d) => break (U16_HEX_BYTES + ENCODED_BAND, d.len()),
BandRef::Progress(d) => {
let text = TextRef::from(d).0;
handle_progress(false, text);
}
Band::Error(d) => {
let text = Text::from(d).0;
BandRef::Error(d) => {
let text = TextRef::from(d).0;
handle_progress(true, text);
}
};
Expand Down
15 changes: 6 additions & 9 deletions git-packetline/src/read/sidebands/blocking_io.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use std::{io, io::BufRead};

use crate::{
immutable::{Band, Text},
PacketLineRef, StreamingPeekableIter, U16_HEX_BYTES,
};
use crate::{BandRef, PacketLineRef, StreamingPeekableIter, TextRef, U16_HEX_BYTES};

/// An implementor of [`BufRead`][io::BufRead] yielding packet lines on each call to [`read_line()`][io::BufRead::read_line()].
/// It's also possible to hide the underlying packet lines using the [`Read`][io::Read] implementation which is useful
Expand Down Expand Up @@ -116,13 +113,13 @@ where
.map_err(|err| io::Error::new(io::ErrorKind::Other, err))?;
const ENCODED_BAND: usize = 1;
match band {
Band::Data(d) => break (U16_HEX_BYTES + ENCODED_BAND, d.len()),
Band::Progress(d) => {
let text = Text::from(d).0;
BandRef::Data(d) => break (U16_HEX_BYTES + ENCODED_BAND, d.len()),
BandRef::Progress(d) => {
let text = TextRef::from(d).0;
handle_progress(false, text);
}
Band::Error(d) => {
let text = Text::from(d).0;
BandRef::Error(d) => {
let text = TextRef::from(d).0;
handle_progress(true, text);
}
};
Expand Down
5 changes: 2 additions & 3 deletions git-packetline/tests/decode/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
mod streaming {
use git_packetline::{
decode::{self, streaming, Stream},
immutable::Error,
PacketLineRef,
ErrorRef, PacketLineRef,
};

use crate::assert_err_display;
Expand Down Expand Up @@ -118,7 +117,7 @@ mod streaming {
)?;
assert_eq!(
line.check_error().expect("error to be parsed here"),
Error(b"the error")
ErrorRef(b"the error")
);
Ok(())
}
Expand Down
6 changes: 1 addition & 5 deletions git-transport/src/client/async_io/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,7 @@ impl<'a> RequestWriter<'a> {
.write_to(self.writer.inner_mut())
.await
}
MessageKind::Text(t) => {
git_packetline::immutable::Text::from(t)
.write_to(self.writer.inner_mut())
.await
}
MessageKind::Text(t) => git_packetline::TextRef::from(t).write_to(self.writer.inner_mut()).await,
}
.map(|_| ())
}
Expand Down
2 changes: 1 addition & 1 deletion git-transport/src/client/blocking_io/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl<'a> RequestWriter<'a> {
MessageKind::Flush => git_packetline::PacketLineRef::Flush.write_to(self.writer.inner_mut()),
MessageKind::Delimiter => git_packetline::PacketLineRef::Delimiter.write_to(self.writer.inner_mut()),
MessageKind::ResponseEnd => git_packetline::PacketLineRef::ResponseEnd.write_to(self.writer.inner_mut()),
MessageKind::Text(t) => git_packetline::immutable::Text::from(t).write_to(self.writer.inner_mut()),
MessageKind::Text(t) => git_packetline::TextRef::from(t).write_to(self.writer.inner_mut()),
}
.map(|_| ())
}
Expand Down
2 changes: 1 addition & 1 deletion git-transport/src/client/capabilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl Capabilities {
impl Capabilities {
fn extract_protocol<'a>(
capabilities_or_version: &'a git_packetline::PacketLineRef<'_>,
) -> Result<(git_packetline::immutable::Text<'a>, Protocol), client::Error> {
) -> Result<(git_packetline::TextRef<'a>, Protocol), client::Error> {
let first_line = capabilities_or_version
.as_text()
.ok_or(client::Error::ExpectedLine("text"))?;
Expand Down

0 comments on commit 23438fd

Please sign in to comment.