From ddc3a5c04a5b704b6194656d41663b19ffa207e5 Mon Sep 17 00:00:00 2001
From: Erich Gubler <erichdongubler@gmail.com>
Date: Mon, 1 Jun 2020 12:30:27 -0600
Subject: [PATCH] BREAKING CHANGE: improvement: change byte counts to be a
 positive `i64` (with newtype `PositiveI64`) instead of a `u64` to take
 advantage of new skip impl for `--seek` with non-`Seek` `Input`s like STDIN

This is technically a breaking change because it halves the upper range
of accepted values for byte counts. I thought that fine under the
assumption it will be rare for users to ever want to specify something
in the upper half of `u64`'s value range, especially considering that
this can pave the way for having negative byte offsets (instead of just
counts).
---
 src/bin/hexyl.rs | 81 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 59 insertions(+), 22 deletions(-)

diff --git a/src/bin/hexyl.rs b/src/bin/hexyl.rs
index 590ea79..9590e89 100644
--- a/src/bin/hexyl.rs
+++ b/src/bin/hexyl.rs
@@ -1,6 +1,7 @@
 #[macro_use]
 extern crate clap;
 
+use std::convert::TryFrom;
 use std::fs::File;
 use std::io::{self, prelude::*, SeekFrom};
 
@@ -103,15 +104,15 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
 
     let block_size = matches
         .value_of("block_size")
-        .and_then(|bs| bs.parse::<u64>().ok())
-        .unwrap_or(512);
+        .and_then(|bs| bs.parse().ok().and_then(PositiveI64::new))
+        .unwrap_or_else(|| PositiveI64::new(512).unwrap());
 
     let skip_arg = matches
         .value_of("skip")
         .and_then(|s| parse_byte_count(s, block_size));
 
-    if let Some(skip) = skip_arg {
-        reader.seek(SeekFrom::Start(skip))?;
+    if let Some(skip) = skip_arg.clone() {
+        reader.seek(SeekFrom::Current(skip.into_inner()))?;
     }
 
     let length_arg = matches
@@ -120,7 +121,7 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
 
     let mut reader = if let Some(length) = length_arg.and_then(|s| parse_byte_count(s, block_size))
     {
-        Box::new(reader.take(length))
+        Box::new(reader.take(length.into()))
     } else {
         reader.into_inner()
     };
@@ -143,7 +144,8 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
         .value_of("display_offset")
         .and_then(|s| parse_byte_count(s, block_size))
         .or(skip_arg)
-        .unwrap_or(0);
+        .unwrap_or_default()
+        .into();
 
     let stdout = io::stdout();
     let mut stdout_lock = stdout.lock();
@@ -180,23 +182,49 @@ fn main() {
     }
 }
 
-fn parse_byte_count(n: &str, block_size: u64) -> Option<u64> {
+#[derive(Clone, Copy, Debug, Default, Hash, Eq, Ord, PartialEq, PartialOrd)]
+pub struct PositiveI64(i64);
+
+impl PositiveI64 {
+    pub fn new(x: i64) -> Option<Self> {
+        if x.is_negative() {
+            None
+        } else {
+            Some(Self(x))
+        }
+    }
+
+    pub fn into_inner(self) -> i64 {
+        self.0
+    }
+}
+
+impl Into<u64> for PositiveI64 {
+    fn into(self) -> u64 {
+        u64::try_from(self.0)
+            .expect("invariant broken: PositiveI64 should contain a positive i64 value")
+    }
+}
+
+fn parse_byte_count(n: &str, block_size: PositiveI64) -> Option<PositiveI64> {
     const HEX_PREFIX: &'static str = "0x";
 
     let n = {
         let mut chars = n.chars();
         match chars.next()? {
             '+' => chars.as_str(),
+            '-' => return None,
             _ => n,
         }
     };
 
     if n.starts_with(HEX_PREFIX) {
         let n = &n[HEX_PREFIX.len()..];
-        if n.chars().next() == Some('+') {
-            return None;
+        match n.chars().next() {
+            Some('+') | Some('-') => return None,
+            _ => (),
         }
-        return u64::from_str_radix(n, 16).ok();
+        return i64::from_str_radix(n, 16).ok().and_then(PositiveI64::new);
     }
 
     let (n, unit_multiplier) = match n.chars().position(|c| !c.is_ascii_digit()) {
@@ -207,15 +235,15 @@ fn parse_byte_count(n: &str, block_size: u64) -> Option<u64> {
                 n,
                 [
                     ("b", 1),
-                    ("kb", 1000u64.pow(1)),
-                    ("mb", 1000u64.pow(2)),
-                    ("gb", 1000u64.pow(3)),
-                    ("tb", 1000u64.pow(4)),
-                    ("kib", 1024u64.pow(1)),
-                    ("mib", 1024u64.pow(2)),
-                    ("gib", 1024u64.pow(3)),
-                    ("tib", 1024u64.pow(4)),
-                    ("block", block_size),
+                    ("kb", 1000i64.pow(1)),
+                    ("mb", 1000i64.pow(2)),
+                    ("gb", 1000i64.pow(3)),
+                    ("tb", 1000i64.pow(4)),
+                    ("kib", 1024i64.pow(1)),
+                    ("mib", 1024i64.pow(2)),
+                    ("gib", 1024i64.pow(3)),
+                    ("tib", 1024i64.pow(4)),
+                    ("block", block_size.into_inner()),
                 ]
                 .iter()
                 .cloned()
@@ -231,7 +259,10 @@ fn parse_byte_count(n: &str, block_size: u64) -> Option<u64> {
         None => (n, 1),
     };
 
-    n.parse::<u64>().ok()?.checked_mul(unit_multiplier)
+    n.parse::<i64>()
+        .ok()?
+        .checked_mul(unit_multiplier)
+        .and_then(PositiveI64::new)
 }
 
 #[test]
@@ -241,13 +272,19 @@ fn test_parse_byte_count() {
             success!($input, 512, $expected)
         };
         ($input: expr, $block_size: expr, $expected: expr) => {
-            assert_eq!(parse_byte_count($input, $block_size), Some($expected));
+            assert_eq!(
+                parse_byte_count($input, PositiveI64::new($block_size).unwrap()),
+                Some(PositiveI64::new($expected).unwrap())
+            );
         };
     }
 
     macro_rules! error {
         ($input: expr) => {
-            assert_eq!(parse_byte_count($input, 512), None);
+            assert_eq!(
+                parse_byte_count($input, PositiveI64::new(512).unwrap()),
+                None
+            );
         };
     }