Skip to content

Normalize labels before addition or removal #2128

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

Merged
merged 6 commits into from
Aug 1, 2025
Merged
Changes from 3 commits
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
145 changes: 108 additions & 37 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,21 +565,12 @@ impl IssueRepository {
format!("{}/{}", self.organization, self.repository)
}

async fn has_label(&self, client: &GithubClient, label: &str) -> anyhow::Result<bool> {
#[allow(clippy::redundant_pattern_matching)]
let url = format!("{}/labels/{}", self.url(client), label);
match client.send_req(client.get(&url)).await {
Ok(_) => Ok(true),
Err(e) => {
if e.downcast_ref::<reqwest::Error>()
.map_or(false, |e| e.status() == Some(StatusCode::NOT_FOUND))
{
Ok(false)
} else {
Err(e)
}
}
}
async fn labels(&self, client: &GithubClient) -> anyhow::Result<Vec<Label>> {
let url = format!("{}/labels", self.url(client));
client
.json(client.get(&url))
.await
.context("failed to get labels")
}
}

Expand All @@ -597,6 +588,30 @@ impl fmt::Display for UnknownLabels {

impl std::error::Error for UnknownLabels {}

#[derive(Debug)]
pub(crate) struct AmbiguousLabelMatch {
pub requested_label: String,
pub labels: Vec<String>,
}

// NOTE: This is used to post the Github comment; make sure it's valid markdown.
impl fmt::Display for AmbiguousLabelMatch {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"Unsure which label to use for `{}` - could be one of: {}",
self.requested_label,
self.labels
.iter()
.map(|l| format!("`{}`", l))
.collect::<Vec<_>>()
.join(", ")
)
}
}

impl std::error::Error for AmbiguousLabelMatch {}

impl Issue {
pub fn to_zulip_github_reference(&self) -> ZulipGitHubReference {
ZulipGitHubReference {
Expand Down Expand Up @@ -730,8 +745,71 @@ impl Issue {
Ok(())
}

async fn normalize_and_match_labels(
&self,
client: &GithubClient,
requested_labels: &[String],
) -> anyhow::Result<Vec<String>> {
let available_labels = self.repository().labels(client).await.unwrap_or_default();

let emoji_regex: Regex = Regex::new(r"[\p{Emoji}\p{Emoji_Presentation}]").unwrap();
let normalize = |s: &str| emoji_regex.replace_all(s, "").trim().to_lowercase();

let mut found_labels = Vec::with_capacity(requested_labels.len());
let mut unknown_labels = Vec::new();

for requested_label in requested_labels {
// First look for an exact match
if let Some(found) = available_labels.iter().find(|l| l.name == *requested_label) {
found_labels.push(&found.name);
continue;
}

// Try normalizing requested label (remove emoji, case insensitive, trim whitespace)
let normalized_requested: String = normalize(requested_label);

// Find matching labels by normalized name
let found = available_labels
.iter()
.filter(|l| normalize(&l.name) == normalized_requested)
.collect::<Vec<_>>();

if found.is_empty() {
unknown_labels.push(requested_label.as_str());
} else if found.len() > 1 {
return Err(AmbiguousLabelMatch {
requested_label: requested_label.clone(),
labels: found.into_iter().map(|l| l.name.clone()).collect(),
}
.into());
} else {
found_labels.push(&found.first().unwrap().name);
}
}

if !unknown_labels.is_empty() {
return Err(UnknownLabels {
labels: unknown_labels.into_iter().map(String::from).collect(),
}
.into());
}

Ok(found_labels.into_iter().map(|s| s.clone()).collect())
}

pub async fn remove_label(&self, client: &GithubClient, label: &str) -> anyhow::Result<()> {
log::info!("remove_label from {}: {:?}", self.global_id(), label);

let normalized_labels = self
.normalize_and_match_labels(client, &[label.to_string()])
.await?;
let label = normalized_labels.first().unwrap();
log::info!(
"remove_label from {}: matched label to {:?}",
self.global_id(),
label
);

// DELETE /repos/:owner/:repo/issues/:number/labels/{name}
let url = format!(
"{repo_url}/issues/{number}/labels/{name}",
Expand Down Expand Up @@ -767,6 +845,19 @@ impl Issue {
labels: Vec<Label>,
) -> anyhow::Result<()> {
log::info!("add_labels: {} +{:?}", self.global_id(), labels);

let labels = self
.normalize_and_match_labels(
client,
&labels.into_iter().map(|l| l.name).collect::<Vec<_>>(),
)
.await?;
log::info!(
"add_labels: {} matched requested labels to +{:?}",
self.global_id(),
labels
);

// POST /repos/:owner/:repo/issues/:number/labels
// repo_url = https://api.github.com/repos/Codertocat/Hello-World
let url = format!(
Expand All @@ -778,8 +869,7 @@ impl Issue {
// Don't try to add labels already present on this issue.
let labels = labels
.into_iter()
.filter(|l| !self.labels().contains(&l))
.map(|l| l.name)
.filter(|l| !self.labels().iter().any(|existing| existing.name == *l))
.collect::<Vec<_>>();

log::info!("add_labels: {} filtered to {:?}", self.global_id(), labels);
Expand All @@ -788,32 +878,13 @@ impl Issue {
return Ok(());
}

let mut unknown_labels = vec![];
let mut known_labels = vec![];
for label in labels {
if !self.repository().has_label(client, &label).await? {
unknown_labels.push(label);
} else {
known_labels.push(label);
}
}

if !unknown_labels.is_empty() {
return Err(UnknownLabels {
labels: unknown_labels,
}
.into());
}

#[derive(serde::Serialize)]
struct LabelsReq {
labels: Vec<String>,
}

client
.send_req(client.post(&url).json(&LabelsReq {
labels: known_labels,
}))
.send_req(client.post(&url).json(&LabelsReq { labels }))
.await
.context("failed to add labels")?;

Expand Down