Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt: Replace horrible handwritten parser with a regex #25

Merged
merged 2 commits into from
Mar 8, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion tokio-trace-fmt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ ansi = ["ansi_term"]
[dependencies]
tokio-trace-core = { git = "https://github.com/tokio-rs/tokio" }
ansi_term = { version = "0.11", optional = true }
regex = "1"
lazy_static = "1"
owning_ref = "0.4.0"

parking_lot = { version = "0.7"}
lock_api = "0.1"

Expand Down
134 changes: 61 additions & 73 deletions tokio-trace-fmt/src/filter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use span;

use tokio_trace_core::{subscriber::Interest, Level, Metadata};
use regex::Regex;

use std::env;

Expand Down Expand Up @@ -171,21 +172,13 @@ impl Filter for EnvFilter {
}

fn parse_directives(spec: &str) -> Vec<Directive> {
// N.B. that this is based pretty closely on the `env_logger` crate,
// since we want to accept a superset of their syntax. Refer to
// https://github.com/sebasmagri/env_logger/blob/master/src/filter/mod.rs

let mut dirs = Vec::new();

for dir in spec.split(',') {
if let Some(dir) = Directive::parse(dir) {
dirs.push(dir);
} else {
spec.split(',').filter_map(|dir| {
Directive::parse(dir).or_else(|| {
eprintln!("ignoring invalid log directive '{}'", dir);
}
}

dirs
None
})
})
.collect()
}

// ===== impl Directive =====
Expand All @@ -200,6 +193,21 @@ impl Directive {
}

fn parse(from: &str) -> Option<Self> {
lazy_static! {
static ref DIRECTIVE_RE: Regex = Regex::new(r"(?x)
^(?P<global_level>trace|TRACE|debug|DEBUG|info|INFO|warn|WARN|error|ERROR|[0-5])$ |
^
(?: # target name or span name
(?P<target>[\w:]+)|(?P<span>\[[^\]]*\])
){1,2}
(?: # level or nothing
=(?P<level>trace|TRACE|debug|DEBUG|info|INFO|warn|WARN|error|ERROR|[0-5])?
)?
$
"
).unwrap();
}

fn parse_level(from: &str) -> Option<Level> {
// TODO: maybe this whole function ought to be replaced by a
// `FromStr` impl for `Level` in `tokio_trace_core`...?
Expand All @@ -224,61 +232,41 @@ impl Directive {
})
}

fn parse_span_target(from: &str) -> Option<(Option<String>, Option<String>)> {
let mut parts = from.split('[');
let target = parts
.next()
.and_then(|part| if part.len() == 0 { None } else { Some(part) });
let span_part = parts.next();
if parts.next().is_some() {
return None;
}
let in_span = if let Some(part) = span_part {
let mut parts = part.split(']');
let (part0, part1) = (parts.next(), parts.next());
if part1 != Some("") {
return None;
}
part0
} else {
None
};
Some((target.map(String::from), in_span.map(String::from)))
}
let caps = DIRECTIVE_RE.captures(from)?;

if from.len() == 0 {
return None;
}
let mut parts = from.split('=');
let parse = (parts.next()?, parts.next().map(|s| s.trim()));
if parts.next().is_some() {
return None;
if let Some(level) = caps.name("global_level")
.and_then(|c| parse_level(c.as_str()))
{
return Some(Directive {
level,
..Default::default()
});
}
match parse {
(part0, None) => Some(if let Some(level) = parse_level(part0) {
Directive {
level,
..Default::default()
}
} else {
let (target, in_span) = parse_span_target(part0)?;
Directive {
target,
in_span,
..Default::default()

let target = caps.name("target")
.and_then(|c| {
let s = c.as_str();
if parse_level(s).is_some() {
None
} else {
Some(s.to_owned())
}
}),
(part0, Some(part1)) => {
let (target, in_span) = parse_span_target(part0)?;
let level = parse_level(part1)?;

Some(Directive {
level,
target,
in_span,
})
}
}
});

let in_span = caps.name("span")
.map(|c| c.as_str()
.trim_matches(|c| c == '[' || c == ']')
.to_owned()
);
let level = caps.name("level")
.and_then(|c| parse_level(c.as_str()))
.unwrap_or(Level::ERROR);

Some(Directive {
level,
target,
in_span,
})
}
}

Expand Down Expand Up @@ -342,7 +330,7 @@ mod tests {
#[test]
fn parse_directives_valid() {
let dirs = parse_directives("crate1::mod1=error,crate1::mod2,crate2=debug");
assert_eq!(dirs.len(), 3);
assert_eq!(dirs.len(), 3, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, Some("crate1::mod1".to_string()));
assert_eq!(dirs[0].level, Level::ERROR);
assert_eq!(dirs[0].in_span, None);
Expand All @@ -360,7 +348,7 @@ mod tests {
fn parse_directives_invalid_crate() {
// test parse_directives with multiple = in specification
let dirs = parse_directives("crate1::mod1=warn=info,crate2=debug");
assert_eq!(dirs.len(), 1);
assert_eq!(dirs.len(), 1, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, Some("crate2".to_string()));
assert_eq!(dirs[0].level, Level::DEBUG);
assert_eq!(dirs[0].in_span, None);
Expand All @@ -370,7 +358,7 @@ mod tests {
fn parse_directives_invalid_level() {
// test parse_directives with 'noNumber' as log level
let dirs = parse_directives("crate1::mod1=noNumber,crate2=debug");
assert_eq!(dirs.len(), 1);
assert_eq!(dirs.len(), 1, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, Some("crate2".to_string()));
assert_eq!(dirs[0].level, Level::DEBUG);
assert_eq!(dirs[0].in_span, None);
Expand All @@ -380,7 +368,7 @@ mod tests {
fn parse_directives_string_level() {
// test parse_directives with 'warn' as log level
let dirs = parse_directives("crate1::mod1=wrong,crate2=warn");
assert_eq!(dirs.len(), 1);
assert_eq!(dirs.len(), 1, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, Some("crate2".to_string()));
assert_eq!(dirs[0].level, Level::WARN);
assert_eq!(dirs[0].in_span, None);
Expand All @@ -390,7 +378,7 @@ mod tests {
fn parse_directives_empty_level() {
// test parse_directives with '' as log level
let dirs = parse_directives("crate1::mod1=wrong,crate2=");
assert_eq!(dirs.len(), 1);
assert_eq!(dirs.len(), 1, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, Some("crate2".to_string()));
assert_eq!(dirs[0].level, Level::ERROR);
assert_eq!(dirs[0].in_span, None);
Expand All @@ -400,7 +388,7 @@ mod tests {
fn parse_directives_global() {
// test parse_directives with no crate
let dirs = parse_directives("warn,crate2=debug");
assert_eq!(dirs.len(), 2);
assert_eq!(dirs.len(), 2, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, None);
assert_eq!(dirs[0].level, Level::WARN);
assert_eq!(dirs[1].in_span, None);
Expand All @@ -413,7 +401,7 @@ mod tests {
#[test]
fn parse_directives_valid_with_spans() {
let dirs = parse_directives("crate1::mod1[foo]=error,crate1::mod2[bar],crate2[baz]=debug");
assert_eq!(dirs.len(), 3);
assert_eq!(dirs.len(), 3, "\ngot: {:?}", dirs);
assert_eq!(dirs[0].target, Some("crate1::mod1".to_string()));
assert_eq!(dirs[0].level, Level::ERROR);
assert_eq!(dirs[0].in_span, Some("foo".to_string()));
Expand Down
4 changes: 4 additions & 0 deletions tokio-trace-fmt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ extern crate lock_api;
extern crate owning_ref;
extern crate parking_lot;

#[macro_use]
extern crate lazy_static;
extern crate regex;

use tokio_trace_core::{field, subscriber::Interest, Event, Metadata};

use std::{cell::RefCell, fmt, io};
Expand Down