wip: nom error rework + control command rework

src/errors.rs

@@ -4,6 +4,8 @@ use std::io;
 use std::path::PathBuf;
 use thiserror::Error;
 
+use crate::vobsub::NomError;
+
 /// A type representing errors that are specific to `subtile`. Note that we may
 /// normally return `Error`, not `SubError`, which allows to return other
 /// kinds of errors from third-party libraries.
@@ -29,6 +31,10 @@ pub enum SubError {
     #[error("Unexpected extra input")]
     UnexpectedInput,
 
+    /// If an error happen during parsing with `nom`.
+    #[error("Parsing error.")]
+    NomParsing(#[from] NomError),
+
     /// We could not read a file.
     #[error("Could not read '{path}'")]
     Io {

src/vobsub/idx.rs

@@ -93,7 +93,7 @@ where
             let val = cap.get(2).unwrap().as_str();
             match key {
                 "palette" => {
-                    palette_val = Some(palette(val.as_bytes()).to_vobsub_result()?);
+                    palette_val = Some(palette(val.as_bytes()).to_result_no_rest()?);
                 }
                 _ => trace!("Unimplemented idx key: {}", key),
             }

src/vobsub/mod.rs

@@ -72,45 +72,67 @@ pub use self::palette::{palette, Palette};
 pub use self::probe::{is_idx_file, is_sub_file};
 pub use self::sub::{subtitles, Subtitle, Subtitles};
 
-use crate::SubError;
-use core::fmt;
-use nom::IResult;
+use nom::{IResult, Needed};
+use std::fmt;
+use thiserror::Error;
 
-/// Extend `IResult` management, and convert to [`Result`] with [`Subrror`]
+/// Error from `nom` handling
+#[derive(Debug, Error)]
+pub enum NomError {
+    /// We have leftover input that we didn't expect.
+    #[error("Unexpected extra input")]
+    UnexpectedInput,
+
+    /// Our input data ended sooner than we expected.
+    #[error("Incomplete input: '{0:?}' needed.")]
+    IncompleteInput(Needed),
+
+    /// An error happend during parsing
+    #[error("Error from nom : {0}")]
+    Error(String),
+
+    /// And Failure happend during parsing
+    #[error("Failure from nom : {0}")]
+    Failure(String),
+}
+
+/// Extend `IResult` management, and convert to [`Result`] with [`NomError`]
 pub trait IResultExt<I, O, E> {
-    /// Forward IResult after trailing remaining data.
+    /// Convert an `IResult` to Result<_, `NomError`> and check than the buffer is empty after parsing.
     /// # Errors
-    /// Forward `Error` and `Failure` from nom.
-    fn ignore_trailing_data(self) -> IResult<I, O, E>;
-    /// Convert an `IResult` to Result<_, `SubError`>
+    /// Forward `Error` and `Failure` from nom, and return `UnexpectedInput` if the buffer is not empty after parsing.
+    fn to_result_no_rest(self) -> Result<O, NomError>;
+
+    /// Convert an `IResult` to Result<_, `NomError`>
     /// # Errors
-    /// return `UnexpectedInput` if there is trailing data after parsing.
     /// Forward `Error` and `Failure` from nom.
-    fn to_vobsub_result(self) -> Result<O, SubError>;
+    fn to_result(self) -> Result<(I, O), NomError>;
 }
 
 impl<I: Default + Eq, O, E: fmt::Debug> IResultExt<I, O, E> for IResult<I, O, E> {
-    fn ignore_trailing_data(self) -> IResult<I, O, E> {
-        match self {
-            IResult::Ok((_, val)) => IResult::Ok((I::default(), val)),
-            other => other,
-        }
-    }
-
-    fn to_vobsub_result(self) -> Result<O, SubError> {
+    fn to_result_no_rest(self) -> Result<O, NomError> {
         match self {
             IResult::Ok((rest, val)) => {
                 if rest == I::default() {
                     Ok(val)
                 } else {
-                    Err(SubError::UnexpectedInput)
+                    Err(NomError::UnexpectedInput)
                 }
             }
             IResult::Err(err) => match err {
-                nom::Err::Incomplete(_) => Err(SubError::IncompleteInput),
-                nom::Err::Error(err) | nom::Err::Failure(err) => {
-                    Err(SubError::Parse(format!("{err:?}")))
-                }
+                nom::Err::Incomplete(needed) => Err(NomError::IncompleteInput(needed)),
+                nom::Err::Error(err) => Err(NomError::Error(format!("{err:?}"))),
+                nom::Err::Failure(err) => Err(NomError::Failure(format!("{err:?}"))),
+            },
+        }
+    }
+    fn to_result(self) -> Result<(I, O), NomError> {
+        match self {
+            IResult::Ok((rest, val)) => Ok((rest, val)),
+            IResult::Err(err) => match err {
+                nom::Err::Incomplete(needed) => Err(NomError::IncompleteInput(needed)),
+                nom::Err::Error(err) => Err(NomError::Error(format!("{err:?}"))),
+                nom::Err::Failure(err) => Err(NomError::Failure(format!("{err:?}"))),
             },
         }
     }

src/vobsub/sub.rs

@@ -25,6 +25,7 @@ use super::Palette;
 use crate::{
     content::{Area, AreaValues},
     util::BytesFormatter,
+    vobsub::IResultExt,
     SubError,
 };
 use image::{ImageBuffer, Rgba, RgbaImage};
@@ -318,71 +319,58 @@ fn subtitle(raw_data: &[u8], base_time: f64) -> Result<Subtitle, SubError> {
         }
 
         let control_data = &raw_data[control_offset..];
-        match control_sequence(control_data) {
-            IResult::Ok((_, control)) => {
-                trace!("parsed control sequence: {:?}", &control);
-
-                // Extract as much data as we can from this control sequence.
-                let time = base_time + f64::from(control.date) / 100.0;
-                for command in control.commands {
-                    match command {
-                        ControlCommand::Force => {
-                            force = true;
-                        }
-                        ControlCommand::StartDate => {
-                            start_time = start_time.or(Some(time));
-                        }
-                        ControlCommand::StopDate => {
-                            end_time = end_time.or(Some(time));
-                        }
-                        ControlCommand::Palette(p) => {
-                            palette = palette.or(Some(p));
-                        }
-                        ControlCommand::Alpha(a) => {
-                            alpha = alpha.or(Some(a));
-                        }
-                        ControlCommand::Coordinates(c) => {
-                            let cmd_area = Area::try_from(c)?;
-                            area = area.or(Some(cmd_area));
-                        }
-                        ControlCommand::RleOffsets(r) => {
-                            rle_offsets = Some(r);
-                        }
-                        ControlCommand::Unsupported(b) => {
-                            warn!("unsupported control sequence: {:?}", BytesFormatter(b));
-                        }
-                    }
+        let (_, control) = control_sequence(control_data)
+            .to_result()
+            .map_err(|source| SubError::NomParsing(source))?;
+
+        trace!("parsed control sequence: {:?}", &control);
+
+        // Extract as much data as we can from this control sequence.
+        let time = base_time + f64::from(control.date) / 100.0;
+        for command in control.commands {
+            match command {
+                ControlCommand::Force => {
+                    force = true;
                 }
-
-                // Figure out where to look for the next control sequence,
-                // if any.
-                let next_control_offset = cast::usize(control.next);
-                match control_offset.cmp(&next_control_offset) {
-                    Ordering::Greater => {
-                        return Err(SubError::Parse("control offset went backwards".into()));
-                    }
-                    Ordering::Equal => {
-                        // This points back at us, so we're the last packet.
-                        break;
-                    }
-                    Ordering::Less => {
-                        control_offset = next_control_offset;
-                    }
+                ControlCommand::StartDate => {
+                    start_time = start_time.or(Some(time));
                 }
-            }
-            IResult::Err(err) => match err {
-                nom::Err::Incomplete(_) => {
-                    return Err(SubError::Parse("incomplete control packet".into()));
+                ControlCommand::StopDate => {
+                    end_time = end_time.or(Some(time));
+                }
+                ControlCommand::Palette(p) => {
+                    palette = palette.or(Some(p));
+                }
+                ControlCommand::Alpha(a) => {
+                    alpha = alpha.or(Some(a));
                 }
-                nom::Err::Error(err) => {
-                    return Err(SubError::Parse(format!("error parsing subtitle: {err:?}")));
+                ControlCommand::Coordinates(c) => {
+                    let cmd_area = Area::try_from(c)?;
+                    area = area.or(Some(cmd_area));
                 }
-                nom::Err::Failure(err) => {
-                    return Err(SubError::Parse(format!(
-                        "Failure parsing subtitle: {err:?}"
-                    )));
+                ControlCommand::RleOffsets(r) => {
+                    rle_offsets = Some(r);
                 }
-            },
+                ControlCommand::Unsupported(b) => {
+                    warn!("unsupported control sequence: {:?}", BytesFormatter(b));
+                }
+            }
+        }
+
+        // Figure out where to look for the next control sequence,
+        // if any.
+        let next_control_offset = cast::usize(control.next);
+        match control_offset.cmp(&next_control_offset) {
+            Ordering::Greater => {
+                return Err(SubError::Parse("control offset went backwards".into()));
+            }
+            Ordering::Equal => {
+                // This points back at us, so we're the last packet.
+                break;
+            }
+            Ordering::Less => {
+                control_offset = next_control_offset;
+            }
         }
     }