CLEANUP: Extraneous TODOs

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name        = "ucg"
-version     = "0.1.1"
+version     = "0.1.2"
 authors     = ["Jeremy Wall <jeremy@marzhillstudios.com>"]
 description = "A configuration generation grammar."
 repository  = "https://github.com/zaphar/ucg"

TODO.md

@@ -24,6 +24,7 @@ Some options here could be:
 
 # Minor Fixes and Polish
 
+* Streaming Parsing
 * Casting between types?
 * Better error messages.
 * Allow trailing commas.

src/ast/mod.rs

@@ -416,8 +416,6 @@ pub struct SelectDef {
     pub pos: Position,
 }
 
-// TODO(jwall): This should have a way of rendering with position information.
-
 /// Adds position information to any type `T`.
 #[derive(Debug, Clone)]
 pub struct Positioned<T> {

src/build/mod.rs

@@ -113,7 +113,6 @@ impl Val {
         )
     }
 
-    // TODO(jwall): Unit Tests for this.
     pub fn equal(&self, target: &Self, pos: Position) -> Result<bool, error::Error> {
         // first we do a type equality comparison
         match (self, target) {
@@ -128,7 +127,6 @@ impl Val {
                     Ok(false)
                 } else {
                     for (i, v) in ldef.iter().enumerate() {
-                        // TODO(jwall): We should probably do a slightly better error message here.
                         try!(v.equal(lldef[i].as_ref(), pos.clone()));
                     }
                     Ok(true)
@@ -140,16 +138,10 @@ impl Val {
                 } else {
                     for (i, v) in ldef.iter().enumerate() {
                         let field_target = &lldef[i];
-                        eprintln!(
-                            "left field: '{}', right field: '{}'",
-                            v.0.val, field_target.0.val
-                        );
                         if v.0.val != field_target.0.val {
                             // field name equality
-                            eprintln!("Field Not equal!!!");
                             return Ok(false);
                         } else {
-                            eprintln!("Field Equal!!!");
                             // field value equality.
                             if !try!(v.1.equal(field_target.1.as_ref(), v.0.pos.clone())) {
                                 return Ok(false);
@@ -353,7 +345,6 @@ impl Builder {
 
     /// Constructs a new Builder.
     pub fn new() -> Self {
-        // TODO(jwall): Construct a map with the environment variables in it.
         Self::new_with_scope(HashMap::new())
     }
 
@@ -418,7 +409,6 @@ impl Builder {
     pub fn build_file(&mut self, name: &str) -> BuildResult {
         let mut f = try!(File::open(name));
         let mut s = String::new();
-        // TODO(jwall): It would be nice to be able to do this while streaming
         try!(f.read_to_string(&mut s));
         self.build_file_string(s)
     }
@@ -519,7 +509,6 @@ impl Builder {
         if let Some(vv) = Self::find_in_fieldlist(next.1, fs) {
             stack.push_back(vv.clone());
         } else {
-            // TODO(jwall): A better error for this would be nice.
             return Err(Box::new(error::Error::new(
                 format!(
                     "Unable to \
@@ -541,12 +530,10 @@ impl Builder {
         next: (&Position, &str),
         elems: &Vec<Rc<Val>>,
     ) -> Result<(), Box<Error>> {
-        // TODO(jwall): better error reporting here would probably be good.
         let idx = try!(next.1.parse::<usize>());
         if idx < elems.len() {
             stack.push_back(elems[idx].clone());
         } else {
-            // TODO(jwall): A better error for this would be nice.
             return Err(Box::new(error::Error::new(
                 format!(
                     "Unable to \
@@ -563,7 +550,6 @@ impl Builder {
 
     fn lookup_selector(&self, sl: &SelectorList) -> Result<Rc<Val>, Box<Error>> {
         let first = try!(self.eval_expr(&sl.head));
-        // TODO(jwall): Handle environment lookups.
         // First we ensure that the result is a tuple or a list.
         let mut stack = VecDeque::new();
         match first.as_ref() {
@@ -904,7 +890,6 @@ impl Builder {
         if let Val::Tuple(ref src_fields) = *v {
             let mut m = HashMap::<Positioned<String>, (i32, Rc<Val>)>::new();
             // loop through fields and build  up a hashmap
-            // TODO(jwall): Maintain field order here.
             let mut count = 0;
             for &(ref key, ref val) in src_fields.iter() {
                 if let Entry::Vacant(v) = m.entry(key.clone()) {
@@ -925,7 +910,6 @@ impl Builder {
             }
             for &(ref key, ref val) in def.fields.iter() {
                 let expr_result = try!(self.eval_expr(val));
-                // TODO(jwall): Maintain field order here.
                 match m.entry(key.into()) {
                     // brand new field here.
                     Entry::Vacant(v) => {
@@ -1092,7 +1076,6 @@ impl Builder {
     // Evals a single Expression in the context of a running Builder.
     // It does not mutate the builders collected state at all.
     pub fn eval_expr(&self, expr: &Expression) -> Result<Rc<Val>, Box<Error>> {
-        // TODO(jwall): We need a rewrite step to handle operator precendence order.
         match expr {
             &Expression::Simple(ref val) => self.value_to_val(val),
             &Expression::Binary(ref def) => self.eval_binary(def),

src/build/test.rs

@@ -377,7 +377,6 @@ fn test_eval_selector_list_expr() {
                 (value_node!("var2".to_string(), 1, 1), Rc::new(Val::Int(1))),
             ])),
         ])));
-    // TODO(jwall): Assert that we can index into lists using dot syntax.
 
     test_expr_to_val(
         vec![

src/convert/env.rs

@@ -35,7 +35,6 @@ impl EnvConverter {
         w: &mut Write,
     ) -> Result<()> {
         for &(ref name, ref val) in flds.iter() {
-            // TODO(jwall): What if the value is a tuple?
             if val.is_tuple() {
                 eprintln!("Skipping embedded tuple...");
                 return Ok(());
@@ -51,7 +50,6 @@ impl EnvConverter {
     }
 
     fn convert_list(&self, _items: &Vec<Rc<Val>>, _w: &mut Write) -> Result<()> {
-        // TODO(jwall)
         eprintln!("Skipping List...");
         Ok(())
     }

src/convert/json.rs

@@ -68,8 +68,6 @@ impl JsonConverter {
             }
             &Val::String(ref s) => serde_json::Value::String(s.clone()),
             &Val::Macro(_) => {
-                // TODO(jwall): We probably want to actually skip this but for now
-                // we'll use null
                 eprintln!("Skipping macro encoding as null...");
                 serde_json::Value::Null
             }

src/error.rs

@@ -94,7 +94,6 @@ impl Error {
     ) -> Self {
         match cause {
             nom::ErrorKind::Custom(e) => Self::new_with_cause(msg, t, pos, e),
-            // TODO(jwall): We could get more creative here with our messaging.
             _ => Self::new(msg, t, pos),
         }
     }

src/main.rs

@@ -54,7 +54,6 @@ fn run_converter(c: ConverterRunner, v: Rc<Val>, f: Option<&str>) -> io::Result<
 }
 
 fn main() {
-    // TODO(jwall): Read and build an actual file.
     let app = do_flags();
     if let Some(matches) = app.subcommand_matches("build") {
         let file = matches.value_of("INPUT").unwrap();

src/parse/mod.rs

@@ -519,7 +519,6 @@ fn tuple_to_macro(mut t: (Position, Vec<Value>, Value)) -> ParseResult<Expressio
             fields: v.val,
             pos: t.0,
         })),
-        // TODO(jwall): Show a better version of the unexpected parsed value.
         val => Err(error::Error::new(
             format!("Expected Tuple Got {:?}", val),
             error::ErrorType::UnexpectedToken,
@@ -651,20 +650,17 @@ fn symbol_or_list(input: TokenIter) -> NomResult<Value> {
         IResult::Incomplete(i) => {
             return IResult::Incomplete(i);
         }
-        IResult::Error(_) => {
+        IResult::Error(_) => match list_value(input) {
-            // TODO(jwall): Still missing some. But we need to avoid recursion
+            IResult::Incomplete(i) => {
-            match list_value(input) {
+                return IResult::Incomplete(i);
-                IResult::Incomplete(i) => {
-                    return IResult::Incomplete(i);
-                }
-                IResult::Error(e) => {
-                    return IResult::Error(e);
-                }
-                IResult::Done(i, val) => {
-                    return IResult::Done(i, val);
-                }
             }
-        }
+            IResult::Error(e) => {
+                return IResult::Error(e);
+            }
+            IResult::Done(i, val) => {
+                return IResult::Done(i, val);
+            }
+        },
         IResult::Done(rest, val) => {
             return IResult::Done(rest, val);
         }
@@ -721,15 +717,14 @@ fn tuple_to_list_op(tpl: (Position, Token, Value, Value)) -> ParseResult<Express
                 pos: pos,
             }));
         }
-        // TODO(jwall): We should print a pretter message than debug formatting here.
         return Err(error::Error::new(
-            format!("Expected a list but got {:?}", list),
+            format!("Expected a list but got {}", list.type_name()),
             error::ErrorType::UnexpectedToken,
             pos,
         ));
     }
     return Err(error::Error::new(
-        format!("Expected a selector but got {:?}", macroname),
+        format!("Expected a selector but got {}", macroname.type_name()),
         error::ErrorType::UnexpectedToken,
         pos,
     ));

src/tokenizer/mod.rs

@@ -59,7 +59,6 @@ fn escapequoted(input: Span) -> nom::IResult<Span, String> {
     return nom::IResult::Incomplete(nom::Needed::Unknown);
 }
 
-// TODO(jwall): Handle escapes
 named!(strtok( Span ) -> Token,
        do_parse!(
            span: position!() >>
@@ -180,8 +179,6 @@ named!(pcttok( Span ) -> Token,
        do_tag_tok!(TokenType::PUNCT, "%")
 );
 
-// TODO(jwall): Comparison operators.
-
 named!(eqeqtok( Span ) -> Token,
        do_tag_tok!(TokenType::PUNCT, "==")
 );