od: fix and add tests for simple format args

src/od/formatteriteminfo.rs

@@ -1,4 +1,6 @@
-#[derive(Copy)]
+use std::fmt;
+
+#[derive(Copy, Eq)]
 pub enum FormatWriter {
     IntWriter(fn(u64, usize, usize) -> String),
     FloatWriter(fn(f64) -> String),
@@ -12,7 +14,39 @@ impl Clone for FormatWriter {
     }
 }
 
-#[derive(Copy, Clone)]
+impl PartialEq for FormatWriter {
+    fn eq(&self, other: &FormatWriter) -> bool {
+        use formatteriteminfo::FormatWriter::*;
+
+        match (self, other) {
+            (&IntWriter(ref a), &IntWriter(ref b)) => a == b,
+            (&FloatWriter(ref a), &FloatWriter(ref b)) => a == b,
+            (&MultibyteWriter(ref a), &MultibyteWriter(ref b)) => *a as usize == *b as usize,
+            _ => false,
+        }
+    }
+}
+
+impl fmt::Debug for FormatWriter {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            &FormatWriter::IntWriter(ref p) => {
+                try!(f.write_str("IntWriter:"));
+                fmt::Pointer::fmt(p, f)
+            },
+            &FormatWriter::FloatWriter(ref p) => {
+                try!(f.write_str("FloatWriter:"));
+                fmt::Pointer::fmt(p, f)
+            },
+            &FormatWriter::MultibyteWriter(ref p) => {
+                try!(f.write_str("MultibyteWriter:"));
+                fmt::Pointer::fmt(&(*p as *const ()), f)
+            },
+        }
+     }
+ }
+
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct FormatterItemInfo {
     pub byte_size: usize,
     pub print_width: usize,      // including a space in front of the text

src/od/od.rs

@@ -66,16 +66,17 @@ pub fn uumain(args: Vec<String>) -> i32 {
     opts.optflagmulti("c", "", "ASCII characters or backslash escapes");
     opts.optflagmulti("d", "", "unsigned decimal 2-byte units");
     opts.optflagmulti("D", "", "unsigned decimal 4-byte units");
-    opts.optflagmulti("o", "", "unsigned decimal 2-byte units");
+    opts.optflagmulti("o", "", "octal 2-byte units");
 
-    opts.optflagmulti("I", "", "decimal 2-byte units");
+    opts.optflagmulti("I", "", "decimal 8-byte units");
-    opts.optflagmulti("L", "", "decimal 2-byte units");
+    opts.optflagmulti("L", "", "decimal 8-byte units");
-    opts.optflagmulti("i", "", "decimal 2-byte units");
+    opts.optflagmulti("i", "", "decimal 4-byte units");
+    opts.optflagmulti("l", "", "decimal 8-byte units");
     opts.optflagmulti("x", "", "hexadecimal 2-byte units");
     opts.optflagmulti("h", "", "hexadecimal 2-byte units");
 
     opts.optflagmulti("O", "", "octal 4-byte units");
-    opts.optflagmulti("s", "", "decimal 4-byte units");
+    opts.optflagmulti("s", "", "decimal 2-byte units");
     opts.optflagmulti("X", "", "hexadecimal 4-byte units");
     opts.optflagmulti("H", "", "hexadecimal 4-byte units");
 

src/od/parse_formats.rs

@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use formatteriteminfo::FormatterItemInfo;
 use prn_int::*;
 use prn_char::*;
@@ -12,52 +13,72 @@ macro_rules! hashmap {
     }}
 }
 
+/// Parses format flags from commandline
+///
+/// getopts, docopt, clap don't seem suitable to parse the commandline
+/// arguments used for formats. In particular arguments can appear
+/// multiple times and the order they appear in, is significant.
+///
+/// arguments like -f, -o, -x can appear separate or combined: -fox
+/// it can also be mixed with non format related flags like -v: -fvox
+/// arguments with parameters like -w16 can only appear at the end: -fvoxw16
+/// parameters of -t/--format specify 1 or more formats.
+/// if -- appears on the commandline, parsing should stop.
 pub fn parse_format_flags(args: &Vec<String>) -> Vec<FormatterItemInfo> {
     // Gather up format flags, we don't use getopts becase we need keep them in order.
-    let flags = args[1..]
-        .iter()
-        .filter_map(|w| match w as &str {
-            "--" => None,
-            o if o.starts_with("-") => Some(&o[1..]),
-            _ => None,
-        })
-        .collect::<Vec<_>>();
 
     // TODO: -t fmts
     let known_formats = hashmap![
-        "a" => FORMAT_ITEM_A,
+    'a' => FORMAT_ITEM_A,
-        "B" => FORMAT_ITEM_OCT16,
+    'B' => FORMAT_ITEM_OCT16,
-        "b" => FORMAT_ITEM_OCT8,
+    'b' => FORMAT_ITEM_OCT8,
-        "c" => FORMAT_ITEM_C,
+    'c' => FORMAT_ITEM_C,
-        "D" => FORMAT_ITEM_DEC32U,
+    'D' => FORMAT_ITEM_DEC32U,
-        "d" => FORMAT_ITEM_DEC16U,
+    'd' => FORMAT_ITEM_DEC16U,
-        "e" => FORMAT_ITEM_F64,
+    'e' => FORMAT_ITEM_F64,
-        "F" => FORMAT_ITEM_F64,
+    'F' => FORMAT_ITEM_F64,
-        "f" => FORMAT_ITEM_F32,
+    'f' => FORMAT_ITEM_F32,
-        "H" => FORMAT_ITEM_HEX32,
+    'H' => FORMAT_ITEM_HEX32,
-        "X" => FORMAT_ITEM_HEX32,
+    'h' => FORMAT_ITEM_HEX16,
-        "o" => FORMAT_ITEM_OCT16,
+    'i' => FORMAT_ITEM_DEC32S,
-        "x" => FORMAT_ITEM_HEX16,
+    'I' => FORMAT_ITEM_DEC64S,
-        "h" => FORMAT_ITEM_HEX16,
+    'L' => FORMAT_ITEM_DEC64S,
-
+    'l' => FORMAT_ITEM_DEC64S,
-        "I" => FORMAT_ITEM_DEC16S,
+    'O' => FORMAT_ITEM_OCT32,
-        "L" => FORMAT_ITEM_DEC16S,
+    'o' => FORMAT_ITEM_OCT16,
-        "i" => FORMAT_ITEM_DEC16S,
+    's' => FORMAT_ITEM_DEC16S,
-
+    'X' => FORMAT_ITEM_HEX32,
-        "O" => FORMAT_ITEM_OCT32,
+    'x' => FORMAT_ITEM_HEX16
-        "s" => FORMAT_ITEM_DEC16U
     ];
 
+    let ignored_arg_opts: HashSet<_> = ['A', 'j', 'N', 'S', 'w'].iter().cloned().collect();
+
     let mut formats = Vec::new();
 
-    for flag in flags.iter() {
+    // args[0] is the name of the binary
-        match known_formats.get(flag) {
+    let mut arg_iter = args.iter().skip(1);
+
+    while let Some(arg) = arg_iter.next() {
+        if arg.starts_with("--") {
+            if arg.len() == 2 {
+                break;
+            }
+        }
+        else if arg.starts_with("-") {
+            let mut flags = arg.chars().skip(1);
+            while let Some(c) = flags.next() {
+                if ignored_arg_opts.contains(&c) {
+                    break;
+                }
+                match known_formats.get(&c) {
                     None => {} // not every option is a format
                     Some(r) => {
                         formats.push(*r)
                     }
                 }
             }
+        }
+    }
 
     if formats.is_empty() {
         formats.push(FORMAT_ITEM_OCT16); // 2 byte octal is the default
@@ -65,3 +86,58 @@ pub fn parse_format_flags(args: &Vec<String>) -> Vec<FormatterItemInfo> {
 
     formats
 }
+
+#[allow(dead_code)]
+pub fn parse_format_flags_str(args_str: &Vec<&'static str>) -> Vec<FormatterItemInfo> {
+    let args = args_str.iter().map(|s| s.to_string()).collect();
+    parse_format_flags(&args)
+}
+
+#[test]
+fn test_no_options() {
+    assert_eq!(parse_format_flags_str(
+        &vec!("od")),
+        vec!(FORMAT_ITEM_OCT16));
+}
+
+#[test]
+fn test_one_option() {
+    assert_eq!(parse_format_flags_str(
+        &vec!("od", "-F")),
+        vec!(FORMAT_ITEM_F64));
+}
+
+#[test]
+fn test_two_separate_options() {
+    assert_eq!(parse_format_flags_str(
+        &vec!("od", "-F", "-x")),
+        vec!(FORMAT_ITEM_F64, FORMAT_ITEM_HEX16));
+}
+
+#[test]
+fn test_two_combined_options() {
+   assert_eq!(parse_format_flags_str(
+       &vec!("od", "-Fx")),
+       vec!(FORMAT_ITEM_F64, FORMAT_ITEM_HEX16));
+}
+
+#[test]
+fn test_ignore_non_format_parameters() {
+   assert_eq!(parse_format_flags_str(
+       &vec!("od", "-d", "-Ax")),
+       vec!(FORMAT_ITEM_DEC16U));
+}
+
+#[test]
+fn test_ignore_separate_parameters() {
+   assert_eq!(parse_format_flags_str(
+       &vec!("od", "-I", "-A", "x")),
+       vec!(FORMAT_ITEM_DEC64S));
+}
+
+#[test]
+fn test_ignore_trailing_vals() {
+   assert_eq!(parse_format_flags_str(
+       &vec!("od", "-D", "--", "-x")),
+       vec!(FORMAT_ITEM_DEC32U));
+}

tests/test_od.rs

@@ -158,7 +158,7 @@ fn test_dec() {
             0000000      0      1      2      3  32767 -32768 -32767
             0000016
             ");
-    let result = new_ucmd!().arg("--endian=little").arg("-i").run_piped_stdin(&input[..]);
+    let result = new_ucmd!().arg("--endian=little").arg("-s").run_piped_stdin(&input[..]);
 
     assert_empty_stderr!(result);
     assert!(result.success);