Skip to content

Fix [mentions] glob matching to not match partial directories #2248

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.

Sign up for GitHub

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

Merged
Kobzol merged 3 commits into from
Jan 9, 2026
Merged

Fix [mentions] glob matching to not match partial directories #2248

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
664ab8f
Fix `[mentions]` glob matching to not match partial directories
Urgau Jan 9, 2026
f5fa24a
Add fast path for simple `[mentions]` path matching
Urgau Jan 9, 2026
9dec610
Add non-path-like prefix test in mentions
Urgau Jan 9, 2026
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
127 changes: 119 additions & 8 deletions src/handlers/mentions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,24 @@ pub(super) async fn handle_input(
}

fn modified_paths_matches(modified_paths: &[&Path], entry: &str) -> Vec<PathBuf> {
// Fast-path if entry has no glob components
if globset::escape(entry) == entry {
let path = Path::new(entry);

// Return paths that starts with entry
return modified_paths
.iter()
.filter(|p| p.starts_with(path))
.map(PathBuf::from)
.collect();
}

// Prepare the glob pattern from the entry.
//
// Note that we add an extra `*` at the end in order to get the "starts with" mode
// that we want.
let pattern = if entry.ends_with('*') {
entry.to_string()
} else {
format!("{entry}*")
};
// We first trim any excess `/` at the end of the pattern and then add an alternate
// to match on the path as is or in any sub-directories.
let pattern = entry.trim_end_matches('/');
let pattern = format!("{pattern}{{,/*}}");

// Create the glob pattern and log an error (should have already been reported to
// the user).
Expand All @@ -177,7 +186,7 @@ fn modified_paths_matches(modified_paths: &[&Path], entry: &str) -> Vec<PathBuf>
{
Ok(pattern) => pattern,
Err(err) => {
tracing::error!("invalid glob pattern for {entry}: {err:?}");
tracing::error!("invalid glob pattern for [mentions.\"{entry}\"]: {err}");
return Vec::new();
}
};
Expand Down Expand Up @@ -405,4 +414,106 @@ mod tests {
]
);
}

#[test]
fn entry_submodule_modified() {
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/tools/cargo"
),
vec![PathBuf::from("src/tools/cargo")]
);
}

#[test]
fn entry_submodule_and_normal_dir_modified() {
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/tools/cargo{,test}"
),
vec![
PathBuf::from("src/tools/cargo"),
PathBuf::from("src/tools/cargotest")
]
);
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/tools/cargo*"
),
vec![
PathBuf::from("src/tools/cargo"),
PathBuf::from("src/tools/cargotest")
]
);
}

#[test]
fn entry_submodule_modified_with_trailing_slash() {
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/tools/cargo/"
),
vec![PathBuf::from("src/tools/cargo")]
);
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/tools/cargo//"
),
vec![PathBuf::from("src/tools/cargo")]
);
}

#[test]
fn entry_submodule_modified_glob() {
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/*/cargo"
),
vec![PathBuf::from("src/tools/cargo")]
);
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/*/cargo/"
),
vec![PathBuf::from("src/tools/cargo")]
);
assert_eq!(
modified_paths_matches(
&[
Path::new("src/tools/cargo"),
Path::new("src/tools/cargotest")
],
"src/*/cargo//"
),
vec![PathBuf::from("src/tools/cargo")]
);
}
}