Clean up unnecessary macro use (#8607)

Some minor code cleanup.

We've accumulated a few macros over the years that arguably don't need
to be macros. This PR removes 4 macros by either:
1. Inlining the macro
2. Replacing the macro with a local function
3. Replacing the macro with a closure
This commit is contained in:
Reilly Wood 2023-03-25 00:17:20 -07:00 committed by GitHub
parent aab31833a2
commit d8478ca690
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 96 additions and 141 deletions

View File

@ -79,29 +79,27 @@ impl Highlighter for NuHighlighter {
}}; }};
} }
macro_rules! add_colored_token { let mut add_colored_token = |shape: &FlatShape, text: String| {
($shape:expr, $text:expr) => { output.push((get_shape_color(shape.to_string(), &self.config), text));
output.push((get_shape_color($shape.to_string(), &self.config), $text)) };
};
}
match shape.1 { match shape.1 {
FlatShape::Garbage => add_colored_token!(shape.1, next_token), FlatShape::Garbage => add_colored_token(&shape.1, next_token),
FlatShape::Nothing => add_colored_token!(shape.1, next_token), FlatShape::Nothing => add_colored_token(&shape.1, next_token),
FlatShape::Binary => add_colored_token!(shape.1, next_token), FlatShape::Binary => add_colored_token(&shape.1, next_token),
FlatShape::Bool => add_colored_token!(shape.1, next_token), FlatShape::Bool => add_colored_token(&shape.1, next_token),
FlatShape::Int => add_colored_token!(shape.1, next_token), FlatShape::Int => add_colored_token(&shape.1, next_token),
FlatShape::Float => add_colored_token!(shape.1, next_token), FlatShape::Float => add_colored_token(&shape.1, next_token),
FlatShape::Range => add_colored_token!(shape.1, next_token), FlatShape::Range => add_colored_token(&shape.1, next_token),
FlatShape::InternalCall => add_colored_token!(shape.1, next_token), FlatShape::InternalCall => add_colored_token(&shape.1, next_token),
FlatShape::External => add_colored_token!(shape.1, next_token), FlatShape::External => add_colored_token(&shape.1, next_token),
FlatShape::ExternalArg => add_colored_token!(shape.1, next_token), FlatShape::ExternalArg => add_colored_token(&shape.1, next_token),
FlatShape::Literal => add_colored_token!(shape.1, next_token), FlatShape::Literal => add_colored_token(&shape.1, next_token),
FlatShape::Operator => add_colored_token!(shape.1, next_token), FlatShape::Operator => add_colored_token(&shape.1, next_token),
FlatShape::Signature => add_colored_token!(shape.1, next_token), FlatShape::Signature => add_colored_token(&shape.1, next_token),
FlatShape::String => add_colored_token!(shape.1, next_token), FlatShape::String => add_colored_token(&shape.1, next_token),
FlatShape::StringInterpolation => add_colored_token!(shape.1, next_token), FlatShape::StringInterpolation => add_colored_token(&shape.1, next_token),
FlatShape::DateTime => add_colored_token!(shape.1, next_token), FlatShape::DateTime => add_colored_token(&shape.1, next_token),
FlatShape::List => { FlatShape::List => {
add_colored_token_with_bracket_highlight!(shape.1, shape.0, next_token) add_colored_token_with_bracket_highlight!(shape.1, shape.0, next_token)
} }
@ -116,17 +114,17 @@ impl Highlighter for NuHighlighter {
add_colored_token_with_bracket_highlight!(shape.1, shape.0, next_token) add_colored_token_with_bracket_highlight!(shape.1, shape.0, next_token)
} }
FlatShape::Filepath => add_colored_token!(shape.1, next_token), FlatShape::Filepath => add_colored_token(&shape.1, next_token),
FlatShape::Directory => add_colored_token!(shape.1, next_token), FlatShape::Directory => add_colored_token(&shape.1, next_token),
FlatShape::GlobPattern => add_colored_token!(shape.1, next_token), FlatShape::GlobPattern => add_colored_token(&shape.1, next_token),
FlatShape::Variable => add_colored_token!(shape.1, next_token), FlatShape::Variable => add_colored_token(&shape.1, next_token),
FlatShape::Flag => add_colored_token!(shape.1, next_token), FlatShape::Flag => add_colored_token(&shape.1, next_token),
FlatShape::Pipe => add_colored_token!(shape.1, next_token), FlatShape::Pipe => add_colored_token(&shape.1, next_token),
FlatShape::And => add_colored_token!(shape.1, next_token), FlatShape::And => add_colored_token(&shape.1, next_token),
FlatShape::Or => add_colored_token!(shape.1, next_token), FlatShape::Or => add_colored_token(&shape.1, next_token),
FlatShape::Redirection => add_colored_token!(shape.1, next_token), FlatShape::Redirection => add_colored_token(&shape.1, next_token),
FlatShape::Custom(..) => add_colored_token!(shape.1, next_token), FlatShape::Custom(..) => add_colored_token(&shape.1, next_token),
FlatShape::MatchPattern => add_colored_token!(shape.1, next_token), FlatShape::MatchPattern => add_colored_token(&shape.1, next_token),
} }
last_seen_span = shape.0.end; last_seen_span = shape.0.end;
} }

View File

@ -20,22 +20,26 @@ impl From<Style> for NuStyle {
} }
fn style_get_attr(s: Style) -> Option<String> { fn style_get_attr(s: Style) -> Option<String> {
macro_rules! check {
($attrs:expr, $b:expr, $c:expr) => {
if $b {
$attrs.push($c);
}
};
}
let mut attrs = String::new(); let mut attrs = String::new();
check!(attrs, s.is_blink, 'l'); if s.is_blink {
check!(attrs, s.is_bold, 'b'); attrs.push('l');
check!(attrs, s.is_dimmed, 'd'); };
check!(attrs, s.is_reverse, 'r'); if s.is_bold {
check!(attrs, s.is_strikethrough, 's'); attrs.push('b');
check!(attrs, s.is_underline, 'u'); };
if s.is_dimmed {
attrs.push('d');
};
if s.is_reverse {
attrs.push('r');
};
if s.is_strikethrough {
attrs.push('s');
};
if s.is_underline {
attrs.push('u');
};
if attrs.is_empty() { if attrs.is_empty() {
None None

View File

@ -21,13 +21,6 @@ pub enum ComputableStyle {
Closure(Value), Closure(Value),
} }
// macro used for adding initial values to the style hashmap
macro_rules! initial {
($a:expr, $b:expr) => {
($a.to_string(), ComputableStyle::Static($b))
};
}
// An alias for the mapping used internally by StyleComputer. // An alias for the mapping used internally by StyleComputer.
pub type StyleMapping = HashMap<String, ComputableStyle>; pub type StyleMapping = HashMap<String, ComputableStyle>;
// //
@ -153,6 +146,12 @@ impl<'a> StyleComputer<'a> {
pub fn from_config(engine_state: &'a EngineState, stack: &'a Stack) -> StyleComputer<'a> { pub fn from_config(engine_state: &'a EngineState, stack: &'a Stack) -> StyleComputer<'a> {
let config = engine_state.get_config(); let config = engine_state.get_config();
macro_rules! initial {
($a:expr, $b:expr) => {
($a.to_string(), ComputableStyle::Static($b))
};
}
// Create the hashmap // Create the hashmap
let mut map: StyleMapping = HashMap::from([ let mut map: StyleMapping = HashMap::from([
initial!("separator", Color::White.normal()), initial!("separator", Color::White.normal()),

View File

@ -187,10 +187,8 @@ fn help_frame_data(
} }
fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Vec<Vec<Value>>) { fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Vec<Vec<Value>>) {
macro_rules! nu_str { fn nu_str(s: &impl ToString) -> Value {
($text:expr) => { Value::string(s.to_string(), NuSpan::unknown())
Value::string($text.to_string(), NuSpan::unknown())
};
} }
let arguments = manual let arguments = manual
@ -198,7 +196,7 @@ fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Ve
.iter() .iter()
.map(|e| Value::Record { .map(|e| Value::Record {
cols: vec![String::from("example"), String::from("description")], cols: vec![String::from("example"), String::from("description")],
vals: vec![nu_str!(e.example), nu_str!(e.description)], vals: vec![nu_str(&e.example), nu_str(&e.description)],
span: NuSpan::unknown(), span: NuSpan::unknown(),
}) })
.collect(); .collect();
@ -213,7 +211,7 @@ fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Ve
.iter() .iter()
.map(|e| Value::Record { .map(|e| Value::Record {
cols: vec![String::from("example"), String::from("description")], cols: vec![String::from("example"), String::from("description")],
vals: vec![nu_str!(e.example), nu_str!(e.description)], vals: vec![nu_str(&e.example), nu_str(&e.description)],
span: NuSpan::unknown(), span: NuSpan::unknown(),
}) })
.collect(); .collect();
@ -231,7 +229,7 @@ fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Ve
String::from("context"), String::from("context"),
String::from("description"), String::from("description"),
], ],
vals: vec![nu_str!(e.code), nu_str!(e.context), nu_str!(e.description)], vals: vec![nu_str(&e.code), nu_str(&e.context), nu_str(&e.description)],
span: NuSpan::unknown(), span: NuSpan::unknown(),
}) })
.collect(); .collect();
@ -249,7 +247,7 @@ fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Ve
.iter() .iter()
.map(|v| Value::Record { .map(|v| Value::Record {
cols: vec![String::from("example"), String::from("description")], cols: vec![String::from("example"), String::from("description")],
vals: vec![nu_str!(v.example), nu_str!(v.description)], vals: vec![nu_str(&v.example), nu_str(&v.description)],
span: NuSpan::unknown(), span: NuSpan::unknown(),
}) })
.collect(); .collect();
@ -266,9 +264,9 @@ fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Ve
String::from("values"), String::from("values"),
], ],
vals: vec![ vals: vec![
nu_str!(o.group), nu_str(&o.group),
nu_str!(o.key), nu_str(&o.key),
nu_str!(o.description), nu_str(&o.description),
values, values,
], ],
span: NuSpan::unknown(), span: NuSpan::unknown(),
@ -280,9 +278,9 @@ fn help_manual_data(manual: &HelpManual, aliases: &[String]) -> (Vec<String>, Ve
span: NuSpan::unknown(), span: NuSpan::unknown(),
}; };
let name = nu_str!(manual.name); let name = nu_str(&manual.name);
let aliases = nu_str!(aliases.join(", ")); let aliases = nu_str(&aliases.join(", "));
let desc = nu_str!(manual.description); let desc = nu_str(&manual.description);
let headers = vec![ let headers = vec![
String::from("name"), String::from("name"),

View File

@ -735,12 +735,7 @@ impl Value {
from_user_input: bool, from_user_input: bool,
) -> Result<Value, ShellError> { ) -> Result<Value, ShellError> {
let mut current = self; let mut current = self;
// TODO remove this
macro_rules! err_or_null {
($e:expr, $span:expr) => {
return Err($e)
};
}
for member in cell_path { for member in cell_path {
// FIXME: this uses a few extra clones for simplicity, but there may be a way // FIXME: this uses a few extra clones for simplicity, but there may be a way
// to traverse the path without them // to traverse the path without them
@ -758,18 +753,9 @@ impl Value {
} else if *optional { } else if *optional {
return Ok(Value::nothing(*origin_span)); // short-circuit return Ok(Value::nothing(*origin_span)); // short-circuit
} else if val.is_empty() { } else if val.is_empty() {
err_or_null!( return Err(ShellError::AccessEmptyContent { span: *origin_span })
ShellError::AccessEmptyContent { span: *origin_span },
*origin_span
)
} else { } else {
err_or_null!( return Err(ShellError::AccessBeyondEnd { max_idx:val.len()-1,span: *origin_span });
ShellError::AccessBeyondEnd {
max_idx: val.len() - 1,
span: *origin_span
},
*origin_span
);
} }
} }
Value::Binary { val, .. } => { Value::Binary { val, .. } => {
@ -778,18 +764,9 @@ impl Value {
} else if *optional { } else if *optional {
return Ok(Value::nothing(*origin_span)); // short-circuit return Ok(Value::nothing(*origin_span)); // short-circuit
} else if val.is_empty() { } else if val.is_empty() {
err_or_null!( return Err(ShellError::AccessEmptyContent { span: *origin_span })
ShellError::AccessEmptyContent { span: *origin_span },
*origin_span
)
} else { } else {
err_or_null!( return Err(ShellError::AccessBeyondEnd { max_idx:val.len()-1,span: *origin_span });
ShellError::AccessBeyondEnd {
max_idx: val.len() - 1,
span: *origin_span
},
*origin_span
);
} }
} }
Value::Range { val, .. } => { Value::Range { val, .. } => {
@ -798,10 +775,9 @@ impl Value {
} else if *optional { } else if *optional {
return Ok(Value::nothing(*origin_span)); // short-circuit return Ok(Value::nothing(*origin_span)); // short-circuit
} else { } else {
err_or_null!( return Err(ShellError::AccessBeyondEndOfStream {
ShellError::AccessBeyondEndOfStream { span: *origin_span }, span: *origin_span
*origin_span });
);
} }
} }
Value::CustomValue { val, .. } => { Value::CustomValue { val, .. } => {
@ -823,19 +799,14 @@ impl Value {
// Records (and tables) are the only built-in which support column names, // Records (and tables) are the only built-in which support column names,
// so only use this message for them. // so only use this message for them.
Value::Record { .. } => { Value::Record { .. } => {
err_or_null!(ShellError::TypeMismatch { return Err(ShellError::TypeMismatch {
err_message: "Can't access record values with a row index. Try specifying a column name instead".into(), err_message:"Can't access record values with a row index. Try specifying a column name instead".into(),
span: *origin_span, }, *origin_span) span: *origin_span,
})
} }
Value::Error { error } => return Err(*error.to_owned()), Value::Error { error } => return Err(*error.to_owned()),
x => { x => {
err_or_null!( return Err(ShellError::IncompatiblePathAccess { type_name:format!("{}",x.get_type()), span: *origin_span })
ShellError::IncompatiblePathAccess {
type_name: format!("{}", x.get_type()),
span: *origin_span
},
*origin_span
)
} }
} }
} }
@ -862,20 +833,14 @@ impl Value {
} else { } else {
if from_user_input { if from_user_input {
if let Some(suggestion) = did_you_mean(&cols, column_name) { if let Some(suggestion) = did_you_mean(&cols, column_name) {
err_or_null!( return Err(ShellError::DidYouMean(suggestion, *origin_span));
ShellError::DidYouMean(suggestion, *origin_span),
*origin_span
);
} }
} }
err_or_null!( return Err(ShellError::CantFindColumn {
ShellError::CantFindColumn { col_name: column_name.to_string(),
col_name: column_name.to_string(), span: *origin_span,
span: *origin_span, src_span: span,
src_span: span });
},
*origin_span
);
} }
} }
Value::LazyRecord { val, span } => { Value::LazyRecord { val, span } => {
@ -888,20 +853,14 @@ impl Value {
} else { } else {
if from_user_input { if from_user_input {
if let Some(suggestion) = did_you_mean(&columns, column_name) { if let Some(suggestion) = did_you_mean(&columns, column_name) {
err_or_null!( return Err(ShellError::DidYouMean(suggestion, *origin_span));
ShellError::DidYouMean(suggestion, *origin_span),
*origin_span
);
} }
} }
err_or_null!( return Err(ShellError::CantFindColumn {
ShellError::CantFindColumn { col_name: column_name.to_string(),
col_name: column_name.to_string(), span: *origin_span,
span: *origin_span, src_span: *span,
src_span: *span });
},
*origin_span
);
} }
} }
// String access of Lists always means Table access. // String access of Lists always means Table access.
@ -951,15 +910,12 @@ impl Value {
Value::Nothing { .. } if *optional => { Value::Nothing { .. } if *optional => {
return Ok(Value::nothing(*origin_span)); // short-circuit return Ok(Value::nothing(*origin_span)); // short-circuit
} }
Value::Error { error } => err_or_null!(*error.to_owned(), *origin_span), Value::Error { error } => return Err(*error.to_owned()),
x => { x => {
err_or_null!( return Err(ShellError::IncompatiblePathAccess {
ShellError::IncompatiblePathAccess { type_name: format!("{}", x.get_type()),
type_name: format!("{}", x.get_type()), span: *origin_span,
span: *origin_span })
},
*origin_span
)
} }
}, },
} }