Merge pull request #1276 from HeroicKatora/help-new-contributors

Add some TODO to mark unidiomatic code

src/dxt.rs

@@ -81,6 +81,9 @@ impl<R: Read> DxtDecoder<R> {
         variant: DXTVariant,
     ) -> Result<DxtDecoder<R>, ImageError> {
         if width % 4 != 0 || height % 4 != 0 {
+            // TODO: this is actually a bit of a weird case. We could return `DecodingError` but
+            // it's not really the format that is wrong However, the encoder should surely return
+            // `EncodingError` so it would be the logical choice for symmetry.
             return Err(ImageError::Parameter(ParameterError::from_kind(
                 ParameterErrorKind::DimensionMismatch,
             )));
@@ -206,6 +209,7 @@ impl<W: Write> DXTEncoder<W> {
         variant: DXTVariant,
     ) -> ImageResult<()> {
         if width % 4 != 0 || height % 4 != 0 {
+            // TODO: this is not very idiomatic yet. Should return an EncodingError.
             return Err(ImageError::Parameter(ParameterError::from_kind(
                 ParameterErrorKind::DimensionMismatch,
             )));

src/farbfeld.rs

@@ -226,11 +226,8 @@ impl<W: Write> FarbfeldEncoder<W> {
     /// Encodes the image ```data``` (native endian)
     /// that has dimensions ```width``` and ```height```
     pub fn encode(self, data: &[u8], width: u32, height: u32) -> ImageResult<()> {
-        self.encode_impl(data, width, height).map_err(|err|
-            ImageError::Encoding(EncodingError::new(
-                ImageFormat::Farbfeld.into(),
-                err,
-            )))
+        self.encode_impl(data, width, height)?;
+        Ok(())
     }
 
     fn encode_impl(mut self, data: &[u8], width: u32, height: u32) -> io::Result<()> {

src/gif.rs

@@ -385,6 +385,7 @@ impl<W: Write> Encoder<W> {
             Some((width, height))
         }
 
+        // TODO: this is not very idiomatic yet. Should return an EncodingError.
         inner_dimensions(width, height).ok_or(ImageError::Parameter(ParameterError::from_kind(
             ParameterErrorKind::DimensionMismatch
         )))

src/ico/encoder.rs

@@ -103,6 +103,8 @@ fn write_direntry<W: Write>(
 /// Encode a width/height value as a single byte, where 0 means 256.
 fn write_width_or_height<W: Write>(w: &mut W, value: u32) -> io::Result<()> {
     if value < 1 || value > 256 {
+        // TODO: this is not very idiomatic yet. Should return an EncodingError and be checked
+        // prior to encoding.
         return Err(io::Error::new(
             io::ErrorKind::InvalidData,
             "Invalid ICO dimensions (width and \

src/jpeg/encoder.rs

@@ -499,6 +499,8 @@ impl<'a, W: Write> JPEGEncoder<'a, W> {
         build_frame_header(
             &mut buf,
             8,
+            // TODO: not idiomatic yet. Should be an EncodingError and mention jpg. Further it
+            // should check dimensions prior to writing.
             u16::try_from(image.width()).map_err(|_| {
                 ImageError::Parameter(ParameterError::from_kind(
                     ParameterErrorKind::DimensionMismatch,
@@ -681,6 +683,7 @@ impl<'a, W: Write> ImageEncoder for JPEGEncoder<'a, W> {
 fn build_jfif_header(m: &mut Vec<u8>, density: PixelDensity) {
     m.clear();
 
+    // TODO: More idiomatic would be extend_from_slice, to_be_bytes
     let _ = write!(m, "JFIF");
     let _ = m.write_all(&[0]);
     let _ = m.write_all(&[0x01]);
@@ -705,6 +708,7 @@ fn build_frame_header(
 ) {
     m.clear();
 
+    // TODO: More idiomatic would be extend_from_slice, to_be_bytes
     let _ = m.write_all(&[precision]);
     let _ = m.write_u16::<BigEndian>(height);
     let _ = m.write_u16::<BigEndian>(width);
@@ -721,6 +725,7 @@ fn build_frame_header(
 fn build_scan_header(m: &mut Vec<u8>, components: &[Component]) {
     m.clear();
 
+    // TODO: More idiomatic would be extend_from_slice, to_be_bytes
     let _ = m.write_all(&[components.len() as u8]);
 
     for &comp in components.iter() {
@@ -744,6 +749,7 @@ fn build_huffman_segment(
 ) {
     m.clear();
 
+    // TODO: More idiomatic would be pub, extend_from_slice
     let tcth = (class << 4) | destination;
     let _ = m.write_u8(tcth);
 
@@ -766,6 +772,7 @@ fn build_quantization_segment(m: &mut Vec<u8>, precision: u8, identifier: u8, qt
     assert_eq!(qtable.len() % 64, 0);
     m.clear();
 
+    // TODO: More idiomatic would be pub, extend_from_slice
     let p = if precision == 8 { 0 } else { 1 };
 
     let pqtq = (p << 4) | identifier;