Text formatting with embedded markup for terminal output

Question

I'm learning Rust. I wrote the following library to format text and output it to terminal, so that the formatting markup is embedded with the string itself (looks similar to HTML4).

I feel the code is pretty unidiomatic and I'd really appreciate hints on how to make it better, more readable and easier to extend.

The parser is ad-hoc and this is intentional - I don't want to depend on external libraries, as they tend to break often now.

Usage example can be found here.

Some specific points:

1. I don't like how some Tokens contain Options inside (e.g. line 42). This means that in case user wants to use second interface of library - passing slice of tokens for rendering - they must specify all Options as containing Some(_), otherwise rendering will panic.
2. I don't like the error handling - rendering will panic in a lot of places if fed with wrong stream of tokens. I believe this is the result of 1, but nevertheless.
3. Propagating the Result, like at line 287 - there must be proper idiomatic way to do this, right?
4. Iteration - lines 63 and 237 - I had to do the work in nested block since otherwise the borrow checker complains about two mutable references to iter. I feel like there's a better way nevertheless.
5. Matching on &Something - line 245 - I had to do this because of borrow checker - it didn't allow to move the value out of immutable slice via iterator. I know matching on Something is preferred, but couldn't figure out the way to do it here.
6. A slightly minor concern is duplication of attributes - I have the same ones in Token as term has in term::attr::*. Looks like another result of point 1.

Second half of the listing is tests and are provided for reference.

extern crate term;

use State::{Beginning, Tag, Inside, InsideColor, InsideBool};
use Token::{Fg, Bg, Bold, Dim, Italic, Underline, Blink, Standout,
            Reverse, Secure,
            Reset,
            Literal};
use term::{Terminal, WriterWrapper};
pub use term::color::Color as Color;
pub use term::color::{
    BLACK,
    BLUE,
    BRIGHT_BLACK,
    BRIGHT_BLUE,
    BRIGHT_CYAN,
    BRIGHT_GREEN,
    BRIGHT_MAGENTA,
    BRIGHT_RED,
    BRIGHT_WHITE,
    BRIGHT_YELLOW,
    CYAN,
    GREEN,
    MAGENTA,
    RED,
    WHITE,
    YELLOW,
};

pub type FullTerminal = Box<Terminal<WriterWrapper> + Send>;

#[deriving(Show)]
enum State {
    Beginning,
    Tag,
    Inside,
    InsideColor,
    InsideBool,
}

#[deriving(Show, PartialEq, Eq)]
pub enum Token {
    Fg(Option<Color>),
    Bg(Option<Color>),
    Bold,
    Dim,
    Italic(Option<bool>),
    Underline(Option<bool>),
    Blink,
    Standout(Option<bool>),
    Reverse,
    Secure,
    Reset,
    Literal(String),
}

fn parse(s: &str) -> Result<Vec<Token>, String> {
    let mut state = Beginning;
    let mut current = String::new();
    let mut tokens = vec![];
    let mut iter = s.chars().peekable();
    loop {
        {
            let n = iter.peek();
            match n {
                None => {
                    match state {
                        Tag => return Err(format!("Expected lowercase letter or '(', found EOF")),
                        Inside => return Err(format!("Expected ')', found EOF")),
                        InsideColor => return Err(format!("Expected ')', found EOF")),
                        InsideBool => return Err(format!("Expected ')', found EOF")),
                        Beginning => if current.as_slice() != "" {
                            tokens.push(Literal(current.clone()))
                        }
                    }
                    break;
                }
                Some(i) =>
                    match state {
                        Beginning => match *i {
                            '^' => {
                                state = Tag;
                                if current.as_slice() != "" {
                                    tokens.push(Literal(current.clone()));
                                }
                                current = String::new();
                            },
                            _   => {
                                state = Beginning;
                                current.grow(1, *i);
                            },
                        },
                        Tag => match *i {
                            'a'...'z' | '-' => {
                                state = Tag;
                                current.grow(1, *i);
                            },
                            '(' => {
                                match current.as_slice() {
                                    "fg" => {
                                        tokens.push(Fg(None));
                                        state = InsideColor;
                                    }
                                    "bg" => {
                                        tokens.push(Bg(None));
                                        state = InsideColor;
                                    }
                                    "bold" => {
                                        tokens.push(Bold);
                                        state = Inside;
                                    }
                                    "dim" => {
                                        tokens.push(Dim);
                                        state = Inside;
                                    }
                                    "italic" => {
                                        tokens.push(Italic(None));
                                        state = InsideBool;
                                    }
                                    "underline" => {
                                        tokens.push(Underline(None));
                                        state = InsideBool;
                                    }
                                    "blink" => {
                                        tokens.push(Blink);
                                        state = Inside;
                                    }
                                    "standout" => {
                                        tokens.push(Standout(None));
                                        state = InsideBool;
                                    }
                                    "reverse" => {
                                        tokens.push(Reverse);
                                        state = Inside;
                                    }
                                    "secure" => {
                                        tokens.push(Secure);
                                        state = Inside;
                                    }
                                    "reset" => {
                                        tokens.push(Reset);
                                        state = Inside;
                                    }
                                    _ => return Err(format!("Expected a tag, found {}", current)),
                                }
                                current = String::new();
                            }
                            _   => {
                                  return Err(format!("Expected lowercase letter or '(', found {}. Current: {}, state: {}, tokens: {}", i, current, state, tokens));
                            }
                        },
                        Inside => {
                            if current.as_slice() != "" {
                                return Err(format!("Expected no arguments, found {}", current));
                            }
                            match *i {
                                ')' => {
                                    state = Beginning;
                                }
                                _   => {
                                    return Err(format!("Expected ')', found {}", *i));
                                }
                            }
                        }
                        InsideBool => {
                            match *i {
                                ')' => {
                                    state = Beginning;
                                    let value = match current.as_slice() {
                                        "true" => true,
                                        "false" => false,
                                        _ => return Err(format!("Expected bool, found {}", current)),
                                    };
                                    let maybe_last = tokens.pop();
                                    current = String::new();
                                    tokens.push(match maybe_last {
                                        None => return Err(format!("Expected a tag token in array, found {}", maybe_last)),
                                        Some(token) => match token {
                                            Italic(_) => Italic(Some(value)),
                                            Underline(_) => Underline(Some(value)),
                                            Standout(_) => Standout(Some(value)),
                                            _ => return Err(format!("Expected tag, found {}", token)),
                                        }
                                    })
                                }
                                _ => {
                                    state = InsideBool;
                                    current.grow(1, *i);
                                },
                            }
                        }
                        InsideColor => {
                            match *i {
                                ')' => {
                                    state = Beginning;
                                    let color = match current.as_slice() {
                                        "black" => term::color::BLACK,
                                        "blue" => term::color::BLUE,
                                        "bright-black" => term::color::BRIGHT_BLACK,
                                        "bright-blue" => term::color::BRIGHT_BLUE,
                                        "bright-cyan" => term::color::BRIGHT_CYAN,
                                        "bright-green" => term::color::BRIGHT_GREEN,
                                        "bright-magenta" => term::color::BRIGHT_MAGENTA,
                                        "bright-red" => term::color::BRIGHT_RED,
                                        "bright-white" => term::color::BRIGHT_WHITE,
                                        "bright-yellow" => term::color::BRIGHT_YELLOW,
                                        "cyan" => term::color::CYAN,
                                        "green" => term::color::GREEN,
                                        "magenta" => term::color::MAGENTA,
                                        "red" => term::color::RED,
                                        "white" => term::color::WHITE,
                                        "yellow" => term::color::YELLOW,

                                        _ => return Err(format!("Expected color name, found {}", current)),
                                    };
                                    let maybe_last = tokens.pop();
                                    tokens.push(match maybe_last {
                                        None => return Err(format!("Expected a tag token in array, found {}", maybe_last)),
                                        Some(token) => {
                                            match token {
                                                Fg(_) => Fg(Some(color)),
                                                Bg(_) => Bg(Some(color)),
                                                _ => unreachable!(),
                                            }
                                        }
                                    });
                                    current = String::new();
                                },
                                _ => {
                                    state = InsideColor;
                                    current.grow(1, *i);
                                },
                            }
                        },
                    }
            }
        }
        iter.next();
    }
    Ok(tokens)
}

pub fn render(term: &mut FullTerminal, tokens: &[Token]) {
    for t in tokens.iter() {
        match t {
            &Literal(ref string) => write!(term, "{}", string).unwrap(),
            &Fg(maybe_color) => {
                term.fg(maybe_color.unwrap()).unwrap();
            }
            &Bg(maybe_color) => {
                term.bg(maybe_color.unwrap()).unwrap();
            }
            &Bold => {
                term.attr(term::attr::Bold).unwrap();
            }
            &Dim => {
                term.attr(term::attr::Dim).unwrap();
            }
            &Italic(maybe_value) => {
                term.attr(term::attr::Italic(maybe_value.unwrap())).unwrap();
            }
            &Underline(maybe_value) => {
                term.attr(term::attr::Underline(maybe_value.unwrap())).unwrap();
            }
            &Blink => {
                term.attr(term::attr::Blink).unwrap();
            }
            &Standout(maybe_value) => {
                term.attr(term::attr::Standout(maybe_value.unwrap())).unwrap();
            }
            &Reverse => {
                term.attr(term::attr::Reverse).unwrap();
            }
            &Secure => {
                term.attr(term::attr::Secure).unwrap();
            }
            &Reset => {
                term.reset().unwrap();
            }
        }
    }
    term.reset().unwrap();
}

pub fn render_str(term: &mut FullTerminal, s: &str) -> Result<(), String> {
    let maybe_tokens = parse(s);
    match maybe_tokens {
        Err(string) => return Err(string),
        Ok(tokens) => {
            Ok(render(term, tokens.as_slice()))
        }
    }
}

#[test]
fn parse_fg_two_colors() {
    let input = "^fg(red)I'm red text ^fg(blue)I am blue";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Fg(Some(term::color::RED)),
                  Literal("I'm red text ".into_string()),
                  Fg(Some(term::color::BLUE)),
                  Literal("I am blue".into_string())]))
}

#[test]
fn parse_fg_colors_bright() {
    let input = "^fg(bright-green)I'm bright green text ^fg(bright-magenta)I am bright magenta";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Fg(Some(term::color::BRIGHT_GREEN)),
                  Literal("I'm bright green text ".into_string()),
                  Fg(Some(term::color::BRIGHT_MAGENTA)),
                  Literal("I am bright magenta".into_string())]))
}

#[test]
fn parse_fg_bg_colors() {
    let input = "^fg(bright-green)^bg(blue)I'm bright green text ^bg(bright-black)^fg(bright-magenta)I am bright magenta";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Fg(Some(term::color::BRIGHT_GREEN)),
                  Bg(Some(term::color::BLUE)),
                  Literal("I'm bright green text ".into_string()),
                  Bg(Some(term::color::BRIGHT_BLACK)),
                  Fg(Some(term::color::BRIGHT_MAGENTA)),
                  Literal("I am bright magenta".into_string())]))
}

#[test]
fn parse_fg_bg_bold_colors() {
    let input = "^fg(bright-green)^bg(blue)^bold()I'm bold bright green text";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Fg(Some(term::color::BRIGHT_GREEN)),
                  Bg(Some(term::color::BLUE)),
                  Bold,
                  Literal("I'm bold bright green text".into_string()),
                  ]))
}

#[test]
fn parse_dim() {
    let input = "^dim()I'm just dim text";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Dim,
                  Literal("I'm just dim text".into_string()),
                  ]))
}

#[test]
fn parse_reset() {
    let input = "^fg(red)I'm just dim text^reset()";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Fg(Some(term::color::RED)),
                  Literal("I'm just dim text".into_string()),
                  Reset,
                  ]))
}

#[test]
fn parse_italic() {
    let input = "^italic(true)I'm just dim text^italic(false)";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Italic(Some(true)),
                  Literal("I'm just dim text".into_string()),
                  Italic(Some(false)),
                  ]))
}

#[test]
fn parse_underline() {
    let input = "^underline(true)I'm underlined text^underline(false)";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Underline(Some(true)),
                  Literal("I'm underlined text".into_string()),
                  Underline(Some(false)),
                  ]))
}

#[test]
fn parse_blink() {
    let input = "^blink()I'm blinking text^reset()";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Blink,
                  Literal("I'm blinking text".into_string()),
                  Reset,
                  ]))
}

#[test]
fn parse_standout() {
    let input = "^standout(true)I'm standing out text^standout(false)";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Standout(Some(true)),
                  Literal("I'm standing out text".into_string()),
                  Standout(Some(false)),
                  ]))
}

#[test]
fn parse_reverse() {
    let input = "^reverse()I'm reversed text^reset()";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Reverse,
                  Literal("I'm reversed text".into_string()),
                  Reset,
                  ]))
}

#[test]
fn parse_secure() {
    let input = "^secure()I'm secure text^reset()";
    println!("{}", parse(input));
    assert!(parse(input)
         == Ok(
             vec![Secure,
                  Literal("I'm secure text".into_string()),
                  Reset,
                  ]))
}

Answer 1

You have a lot of code and a lot of questions. I'll answer them as I have suggestions (and time).

General thoughts

parse is 186 lines long! I found it really difficult to navigate and understand it, much less make changes. I'd suggest trying to break out smaller methods.

Point 1

Why do these tokens have an Option in the first place? I only see one instance where you set it to None, but all your tests set it to Some and you always unwrap it in render. This seems like you always expect it to be present, so I don't understand why you chose an Option.

Edit 1: I think I get it now. You are in the process of reading through your string, so you don't know what the value will be yet. I'd suggest leveraging the type system to help you out here, which might mean more typing (both for the compiler and your hands). As a high-level example, you could have

enum PartialToken {
    Italic,
    Underline,
    // .. snip ..
}

When you get to the value, you can take the partial token and expand it out to the real token:

match partial_token {
    PartialToken::Italic => Token::Italic(value),
    // .. snip ..
}

You might be able to avoid creating a type if you can keep enough state in local vars to construct the whole token in one shot, but the parse method is too hairy for me to experiment with.

Point 2

I agree that the failure cases of render are all spread out. Here's my current suggested rewrite:

// Comment #1
pub fn render(term: &mut FullTerminal, tokens: &[Token]) -> std::io::IoResult<bool> {
    for t in tokens.iter() {
        let r = match *t {
            // Comment #2
            Literal(ref string)    => write!(term, "{}", string).map(|_| true),
            // Comment #3
            Fg(Some(color))        => term.fg(color),
            Bg(Some(color))        => term.bg(color),
            Bold                   => term.attr(attr::Bold),
            Dim                    => term.attr(attr::Dim),
            Italic(Some(value))    => term.attr(attr::Italic(value)),
            Underline(Some(value)) => term.attr(attr::Underline(value)),
            Blink                  => term.attr(attr::Blink),
            Standout(Some(value))  => term.attr(attr::Standout(value)),
            Reverse                => term.attr(attr::Reverse),
            Secure                 => term.attr(attr::Secure),
            Reset                  => term.reset().map(|_| true),
            // Comment #4
            Fg(None)               |
            Bg(None)               |
            Italic(None)           |
            Underline(None)        |
            Standout(None)         => panic!("Expected value missing"),
        };

        try!(r);
    }

    term.reset().map(|_| true)
}

// Comment #5
pub fn render_str(term: &mut FullTerminal, s: &str) -> Result<(), String> {
    parse(s).and_then(|tokens| {
        render(term, tokens.as_slice()).map(|_| ()).map_err(|x| x.to_string())
    })
}

1. Instead of letting render fail on it's own, let's just propagate the error up to someone who can decide how to deal with it.
2. Here we use map to convert the IoResult<()> to an IoResult<bool>. This allows the types to match, without adding too much confusion. term uses true to mean "it worked" or "it's supported", so that seems a close match.
3. We match on the Some case explicitly to avoid calling it maybe_color and having to unwrap it.
4. We deal with all the None cases at once. Here, I'm lazy and so I I panicked, but it would probably make more sense to not return an IoResult but some custom error type that can handle both cases equally well.
5. Again another reason to have a custom error type, I convert from IoError to String and chain the two Results with and_then.

Point 3

You are looking for the try! macro:

pub fn render_str(term: &mut FullTerminal, s: &str) -> Result<(), String> {
    let tokens = try!(parse(s));
    Ok(render(term, tokens.as_slice()))
}

Or maybe map:

pub fn render_str(term: &mut FullTerminal, s: &str) -> Result<(), String> {
    parse(s).map(|tokens| render(term, tokens.as_slice()))
}

Point 4

I'm not sure why you can't use a regular for-loop:

fn parse(s: &str) -> Result<Vec<Token>, String> {
    // .. snip ..
    // Comment #1
    for i in s.chars() {
        match state {
            Beginning => // .. snip ..
        }
    }

    // Comment #2
    match state {
        Tag => // .. snip ..
    }

    Ok(tokens)
}

1. Simply iterate over each character, no need for a peekableiterator, just one-by-one.
2. At the end, after iterator has finished, do all the stuff that was in the None clause.

Point 5

I wouldn't say that matching on Foo is preferred to &Foo, they are each useful at different times. Perhaps the former is less cluttered than the latter? That being said, I was able to dereference the token once and then match on it like:

for t in tokens.iter() {
    match *t {
        Literal(ref string) => // ...

Point 6

The best way I know to fix this would be to simply reference the upstream enum yourself:

pub enum Token {
    Fg(Option<Color>),
    Bg(Option<Color>),
    // replaces Bold, Dim, Italic, Underline, Blink, Standout, Reverse, and Secure
    Attribute(attr::Attr)
    Reset,
    Literal(String),
}

Unfortunately, your Token is marked as #[deriving(Show, PartialEq, Eq)], but Attr isn't. A brief glance at the source for it doesn't immediately suggest why this should be the case. Perhaps this would be a good chance for you to make a contribution to Rust! ^_^