Skip to content

Commit

Permalink
fix: matching newline chars in glob patterns (#207)
Browse files Browse the repository at this point in the history 
* Enables glob patterns to match against newline chars using regex (?ms) options.
* Adopts builder pattern for Pattern struct, primarily for configuring options (e.g., multiline match support, extglob support).
* Cleans up pattern handling code to remove duplicate logic.
* Adds test case ensuring case patterns work correctly against a multiline string.
  • Loading branch information
@reubeno
reubeno authored Oct 16, 2024
1 parent 7976b18 commit 6cf65ed
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 188 deletions.
2 changes: 1 addition & 1 deletion brush-core/src/builtins/help.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl HelpCommand {

let mut found_count = 0;
for (builtin_name, builtin_registration) in get_builtins_sorted_by_name(context) {
if pattern.exactly_matches(builtin_name.as_str(), false)? {
if pattern.exactly_matches(builtin_name.as_str())? {
self.display_help_for_builtin(
context,
builtin_name.as_str(),
Expand Down
16 changes: 8 additions & 8 deletions brush-core/src/completion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,11 @@ impl Spec {
}

if let Some(glob_pattern) = &self.glob_pattern {
let pattern = patterns::Pattern::from(glob_pattern.as_str());
let pattern = patterns::Pattern::from(glob_pattern.as_str())
.set_extended_globbing(shell.options.extended_globbing);

let expansions = pattern.expand(
shell.working_dir.as_path(),
shell.parser_options().enable_extended_globbing,
Some(&patterns::Pattern::accept_all_expand_filter),
)?;

Expand Down Expand Up @@ -901,12 +902,11 @@ fn get_file_completions(shell: &Shell, context: &Context, must_be_dir: bool) ->
let path_filter = |path: &Path| !must_be_dir || shell.get_absolute_path(path).is_dir();

// TODO: Pass through quoting.
patterns::Pattern::from(glob)
.expand(
shell.working_dir.as_path(),
shell.options.extended_globbing,
Some(&path_filter),
)
let pattern =
patterns::Pattern::from(glob).set_extended_globbing(shell.options.extended_globbing);

pattern
.expand(shell.working_dir.as_path(), Some(&path_filter))
.unwrap_or_default()
.into_iter()
.collect()
Expand Down
51 changes: 19 additions & 32 deletions brush-core/src/expansion.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,10 @@ impl<'a> WordExpander<'a> {
.flatten()
.collect();

Ok(patterns::Pattern::from(pattern_pieces))
let pattern = patterns::Pattern::from(pattern_pieces)
.set_extended_globbing(self.parser_options.enable_extended_globbing);

Ok(pattern)
}

async fn basic_expand_regex(
Expand Down Expand Up @@ -496,11 +499,12 @@ impl<'a> WordExpander<'a> {
}

fn expand_pathnames_in_field(&self, field: WordField) -> Vec<String> {
let pattern = patterns::Pattern::from(field.clone());
let pattern = patterns::Pattern::from(field.clone())
.set_extended_globbing(self.parser_options.enable_extended_globbing);

let expansions = pattern
.expand(
self.shell.working_dir.as_path(),
self.parser_options.enable_extended_globbing,
Some(&patterns::Pattern::accept_all_expand_filter),
)
.unwrap_or_default();
Expand Down Expand Up @@ -779,7 +783,6 @@ impl<'a> WordExpander<'a> {
let result = patterns::remove_smallest_matching_suffix(
expanded_parameter.as_str(),
&expanded_pattern,
self.parser_options.enable_extended_globbing,
)?;
Ok(Expansion::from(result.to_owned()))
}
Expand All @@ -794,7 +797,6 @@ impl<'a> WordExpander<'a> {
let result = patterns::remove_largest_matching_suffix(
expanded_parameter.as_str(),
&expanded_pattern,
self.parser_options.enable_extended_globbing,
)?;

Ok(Expansion::from(result.to_owned()))
Expand All @@ -810,7 +812,6 @@ impl<'a> WordExpander<'a> {
let result = patterns::remove_smallest_matching_prefix(
expanded_parameter.as_str(),
&expanded_pattern,
self.parser_options.enable_extended_globbing,
)?;

Ok(Expansion::from(result.to_owned()))
Expand All @@ -826,7 +827,6 @@ impl<'a> WordExpander<'a> {
let result = patterns::remove_largest_matching_prefix(
expanded_parameter.as_str(),
&expanded_pattern,
self.parser_options.enable_extended_globbing,
)?;

Ok(Expansion::from(result.to_owned()))
Expand Down Expand Up @@ -963,7 +963,7 @@ impl<'a> WordExpander<'a> {
let expanded_pattern = self.basic_expand_opt_pattern(&pattern).await?;

transform_expansion(expanded_parameter, |s| {
self.uppercase_first_char(s, &expanded_pattern)
Self::uppercase_first_char(s, &expanded_pattern)
})
}
brush_parser::word::ParameterExpr::UppercasePattern {
Expand All @@ -975,7 +975,7 @@ impl<'a> WordExpander<'a> {
let expanded_pattern = self.basic_expand_opt_pattern(&pattern).await?;

transform_expansion(expanded_parameter, |s| {
self.uppercase_pattern(s.as_str(), &expanded_pattern)
Self::uppercase_pattern(s.as_str(), &expanded_pattern)
})
}
brush_parser::word::ParameterExpr::LowercaseFirstChar {
Expand All @@ -987,7 +987,7 @@ impl<'a> WordExpander<'a> {
let expanded_pattern = self.basic_expand_opt_pattern(&pattern).await?;

Ok(transform_expansion(expanded_parameter, |s| {
self.lowercase_first_char(s, &expanded_pattern)
Self::lowercase_first_char(s, &expanded_pattern)
})?)
}
brush_parser::word::ParameterExpr::LowercasePattern {
Expand All @@ -999,7 +999,7 @@ impl<'a> WordExpander<'a> {
let expanded_pattern = self.basic_expand_opt_pattern(&pattern).await?;

Ok(transform_expansion(expanded_parameter, |s| {
self.lowercase_pattern(s.as_str(), &expanded_pattern)
Self::lowercase_pattern(s.as_str(), &expanded_pattern)
})?)
}
brush_parser::word::ParameterExpr::ReplaceSubstring {
Expand All @@ -1016,11 +1016,12 @@ impl<'a> WordExpander<'a> {
let replacement = replacement.unwrap_or(String::new());
let expanded_replacement = self.basic_expand_to_str(&replacement).await?;

let regex = patterns::pattern_to_regex(
expanded_pattern.as_str(),
let pattern = patterns::Pattern::from(expanded_pattern.as_str())
.set_extended_globbing(self.parser_options.enable_extended_globbing);

let regex = pattern.to_regex(
matches!(match_kind, brush_parser::word::SubstringMatchKind::Prefix),
matches!(match_kind, brush_parser::word::SubstringMatchKind::Suffix),
self.parser_options.enable_extended_globbing,
)?;

transform_expansion(expanded_parameter, |s| {
Expand Down Expand Up @@ -1350,17 +1351,12 @@ impl<'a> WordExpander<'a> {

#[allow(clippy::unwrap_in_result)]
fn uppercase_first_char(
&mut self,
s: String,
pattern: &Option<patterns::Pattern>,
) -> Result<String, error::Error> {
if let Some(first_char) = s.chars().next() {
let applicable = if let Some(pattern) = pattern {
pattern.is_empty()
|| pattern.exactly_matches(
first_char.to_string().as_str(),
self.shell.options.extended_globbing,
)?
pattern.is_empty() || pattern.exactly_matches(first_char.to_string().as_str())?
} else {
true
};
Expand All @@ -1380,17 +1376,12 @@ impl<'a> WordExpander<'a> {

#[allow(clippy::unwrap_in_result)]
fn lowercase_first_char(
&mut self,
s: String,
pattern: &Option<patterns::Pattern>,
) -> Result<String, error::Error> {
if let Some(first_char) = s.chars().next() {
let applicable = if let Some(pattern) = pattern {
pattern.is_empty()
|| pattern.exactly_matches(
first_char.to_string().as_str(),
self.shell.options.extended_globbing,
)?
pattern.is_empty() || pattern.exactly_matches(first_char.to_string().as_str())?
} else {
true
};
Expand All @@ -1409,14 +1400,12 @@ impl<'a> WordExpander<'a> {
}

fn uppercase_pattern(
&mut self,
s: &str,
pattern: &Option<patterns::Pattern>,
) -> Result<String, error::Error> {
if let Some(pattern) = pattern {
if !pattern.is_empty() {
let regex =
pattern.to_regex(false, false, self.parser_options.enable_extended_globbing)?;
let regex = pattern.to_regex(false, false)?;
let result = regex.replace_all(s.as_ref(), |caps: &fancy_regex::Captures| {
caps[0].to_uppercase()
});
Expand All @@ -1430,14 +1419,12 @@ impl<'a> WordExpander<'a> {
}

fn lowercase_pattern(
&mut self,
s: &str,
pattern: &Option<patterns::Pattern>,
) -> Result<String, error::Error> {
if let Some(pattern) = pattern {
if !pattern.is_empty() {
let regex =
pattern.to_regex(false, false, self.parser_options.enable_extended_globbing)?;
let regex = pattern.to_regex(false, false)?;
let result = regex.replace_all(s.as_ref(), |caps: &fancy_regex::Captures| {
caps[0].to_lowercase()
});
Expand Down
16 changes: 10 additions & 6 deletions brush-core/src/extendedtests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ async fn apply_binary_predicate(
shell.trace_command(std::format!("[[ {s} {op} {expanded_right} ]]"))?;
}

pattern.exactly_matches(s.as_str(), shell.options.extended_globbing)
pattern.exactly_matches(s.as_str())
}
ast::BinaryPredicate::StringDoesNotExactlyMatchPattern => {
let s = expansion::basic_expand_word(shell, left).await?;
Expand All @@ -369,7 +369,7 @@ async fn apply_binary_predicate(
shell.trace_command(std::format!("[[ {s} {op} {expanded_right} ]]"))?;
}

let eq = pattern.exactly_matches(s.as_str(), shell.options.extended_globbing)?;
let eq = pattern.exactly_matches(s.as_str())?;
Ok(!eq)
}
}
Expand Down Expand Up @@ -428,12 +428,16 @@ pub(crate) fn apply_binary_predicate_to_strs(
apply_binary_arithmetic_predicate(left, right, |left, right| left >= right),
),
ast::BinaryPredicate::StringExactlyMatchesPattern => {
let pattern = patterns::Pattern::from(right);
pattern.exactly_matches(left, shell.options.extended_globbing)
let pattern = patterns::Pattern::from(right)
.set_extended_globbing(shell.options.extended_globbing);

pattern.exactly_matches(left)
}
ast::BinaryPredicate::StringDoesNotExactlyMatchPattern => {
let pattern = patterns::Pattern::from(right);
let eq = pattern.exactly_matches(left, shell.options.extended_globbing)?;
let pattern = patterns::Pattern::from(right)
.set_extended_globbing(shell.options.extended_globbing);

let eq = pattern.exactly_matches(left)?;
Ok(!eq)
}
_ => error::unimp("unsupported test binary predicate"),
Expand Down
4 changes: 1 addition & 3 deletions brush-core/src/interp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,9 +580,7 @@ impl Execute for ast::CaseClauseCommand {

for pattern in &case.patterns {
let expanded_pattern = expansion::basic_expand_pattern(shell, pattern).await?;
if expanded_pattern
.exactly_matches(expanded_value.as_str(), shell.options.extended_globbing)?
{
if expanded_pattern.exactly_matches(expanded_value.as_str())? {
matches = true;
break;
}
Expand Down
Loading

0 comments on commit 6cf65ed

Please sign in to comment.