Merge pull request #682 from kellda/diff-command

Execute only changed commands

Author: Mark-Simulacrum

parser/src/command.rs

@@ -14,7 +14,7 @@ pub fn find_commmand_start(input: &str, bot: &str) -> Option<usize> {
     input.find(&format!("@{}", bot))
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq)]
 pub enum Command<'a> {
     Relabel(Result<relabel::RelabelCommand, Error<'a>>),
     Assign(Result<assign::AssignCommand, Error<'a>>),
@@ -197,6 +197,24 @@ fn code_2() {
     assert!(input.parse_command().is_none());
 }
 
+#[test]
+fn edit_1() {
+    let input_old = "@bot modify labels: +bug.";
+    let mut input_old = Input::new(input_old, "bot");
+    let input_new = "Adding labels: @bot modify labels: +bug. some other text";
+    let mut input_new = Input::new(input_new, "bot");
+    assert_eq!(input_old.parse_command(), input_new.parse_command());
+}
+
+#[test]
+fn edit_2() {
+    let input_old = "@bot modify label: +bug.";
+    let mut input_old = Input::new(input_old, "bot");
+    let input_new = "@bot modify labels: +bug.";
+    let mut input_new = Input::new(input_new, "bot");
+    assert_ne!(input_old.parse_command(), input_new.parse_command());
+}
+
 #[test]
 fn move_input_along() {
     let input = "@bot modify labels: +bug. Afterwards, delete the world.";

parser/src/command/relabel.rs

@@ -29,7 +29,7 @@ use crate::token::{Token, Tokenizer};
 use std::error::Error as _;
 use std::fmt;
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq)]
 pub struct RelabelCommand(pub Vec<LabelDelta>);
 
 #[derive(Debug, PartialEq, Eq)]

src/github.rs

@@ -541,6 +541,16 @@ impl Issue {
     }
 }
 
+#[derive(Debug, serde::Deserialize)]
+pub struct ChangeInner {
+    pub from: String,
+}
+
+#[derive(Debug, serde::Deserialize)]
+pub struct Changes {
+    pub body: ChangeInner,
+}
+
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 #[serde(rename_all = "lowercase")]
 pub enum PullRequestReviewAction {
@@ -554,12 +564,14 @@ pub struct PullRequestReviewEvent {
     pub action: PullRequestReviewAction,
     pub pull_request: Issue,
     pub review: Comment,
+    pub changes: Option<Changes>,
     pub repository: Repository,
 }
 
 #[derive(Debug, serde::Deserialize)]
 pub struct PullRequestReviewComment {
     pub action: IssueCommentAction,
+    pub changes: Option<Changes>,
     #[serde(rename = "pull_request")]
     pub issue: Issue,
     pub comment: Comment,
@@ -577,6 +589,7 @@ pub enum IssueCommentAction {
 #[derive(Debug, serde::Deserialize)]
 pub struct IssueCommentEvent {
     pub action: IssueCommentAction,
+    pub changes: Option<Changes>,
     pub issue: Issue,
     pub comment: Comment,
     pub repository: Repository,
@@ -612,6 +625,7 @@ pub struct IssuesEvent {
     pub action: IssuesAction,
     #[serde(alias = "pull_request")]
     pub issue: Issue,
+    pub changes: Option<Changes>,
     pub repository: Repository,
     /// Some if action is IssuesAction::Labeled, for example
     pub label: Option<Label>,
@@ -769,6 +783,14 @@ impl Event {
         }
     }
 
+    /// This will both extract from IssueComment events but also Issue events
+    pub fn comment_from(&self) -> Option<&str> {
+        match self {
+            Event::Issue(e) => Some(&e.changes.as_ref()?.body.from),
+            Event::IssueComment(e) => Some(&e.changes.as_ref()?.body.from),
+        }
+    }
+
     pub fn html_url(&self) -> Option<&str> {
         match self {
             Event::Issue(e) => Some(&e.issue.html_url),

src/handlers/assign.rs

@@ -47,16 +47,29 @@ impl Handler for AssignmentHandler {
         };
 
         if let Event::Issue(e) = event {
-            if e.action != github::IssuesAction::Opened {
+            if !matches!(e.action, github::IssuesAction::Opened | github::IssuesAction::Edited) {
                 log::debug!("skipping event, issue was {:?}", e.action);
-                // skip events other than opening the issue to avoid retriggering commands in the
+                // skip events other than opening or editing the issue to avoid retriggering commands in the
                 // issue body
                 return Ok(None);
             }
         }
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                log::info!("skipping unmodified command: {:?} -> {:?}", prev_command, command);
+                return Ok(None);
+            } else {
+                log::debug!("executing modified command: {:?} -> {:?}", prev_command, command);
+            }
+        }
+        
+        match command {
             Command::Assign(Ok(command)) => Ok(Some(command)),
             Command::Assign(Err(err)) => {
                 return Err(format!(

src/handlers/glacier.rs

@@ -32,7 +32,17 @@ impl Handler for GlacierHandler {
         };
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                return Ok(None);
+            }
+        }
+        
+        match command {
             Command::Glacier(Ok(command)) => Ok(Some(command)),
             Command::Glacier(Err(err)) => {
                 return Err(format!(

src/handlers/major_change.rs

@@ -63,15 +63,21 @@ impl Handler for MajorChangeHandler {
                 // All other issue events are ignored
                 return Ok(None);
             }
-            Event::IssueComment(e) => {
-                if e.action != github::IssueCommentAction::Created {
-                    return Ok(None);
-                }
-            }
+            Event::IssueComment(e) => {}
         }
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                return Ok(None);
+            }
+        }
+        
+        match command {
             Command::Second(Ok(SecondCommand)) => Ok(Some(Invocation::Second)),
             _ => Ok(None),
         }
@@ -107,14 +113,6 @@ async fn handle_input(
                 return Ok(());
             }
 
-            if !issue.labels().iter().any(|l| l.name == "major-change") {
-                let cmnt = ErrorComment::new(
-                    &issue,
-                    "This is not a major change (it lacks the `major-change` label).",
-                );
-                cmnt.post(&ctx.github).await?;
-                return Ok(());
-            }
             let is_team_member =
                 if let Err(_) | Ok(false) = event.user().is_team_member(&ctx.github).await {
                     false

src/handlers/nominate.rs

@@ -30,15 +30,25 @@ impl Handler for NominateHandler {
         };
 
         if let Event::Issue(e) = event {
-            if e.action != github::IssuesAction::Opened {
-                // skip events other than opening the issue to avoid retriggering commands in the
+            if !matches!(e.action, github::IssuesAction::Opened | github::IssuesAction::Edited) {
+                // skip events other than opening or editing the issue to avoid retriggering commands in the
                 // issue body
                 return Ok(None);
             }
         }
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                return Ok(None);
+            }
+        }
+        
+        match command {
             Command::Nominate(Ok(command)) => Ok(Some(command)),
             Command::Nominate(Err(err)) => {
                 return Err(format!(

src/handlers/ping.rs

@@ -33,25 +33,26 @@ impl Handler for PingHandler {
             return Ok(None);
         };
 
-        match event {
-            Event::Issue(e) => {
-                if e.action != github::IssuesAction::Opened {
-                    // skip events other than opening the issue to avoid retriggering commands in the
-                    // issue body
-                    return Ok(None);
-                }
-            }
-            Event::IssueComment(e) => {
-                // Especially on ping commands, which ping tons of folks, this
-                // is quite noisy.
-                if e.action != github::IssueCommentAction::Created {
-                    return Ok(None);
-                }
+        if let Event::Issue(e) = event {
+            if !matches!(e.action, github::IssuesAction::Opened | github::IssuesAction::Edited) {
+                // skip events other than opening or editing the issue to avoid retriggering commands in the
+                // issue body
+                return Ok(None);
             }
         }
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                return Ok(None);
+            }
+        }
+        
+        match command {
             Command::Ping(Ok(command)) => Ok(Some(command)),
             Command::Ping(Err(err)) => {
                 return Err(format!(

src/handlers/prioritize.rs

@@ -25,9 +25,27 @@ impl Handler for PrioritizeHandler {
             // not interested in other events
             return Ok(None);
         };
+        
+        if let Event::Issue(e) = event {
+            if !matches!(e.action, github::IssuesAction::Opened | github::IssuesAction::Edited) {
+                // skip events other than opening or editing the issue to avoid retriggering commands in the
+                // issue body
+                return Ok(None);
+            }
+        }
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                return Ok(None);
+            }
+        }
+        
+        match command {
             Command::Prioritize(Ok(PrioritizeCommand)) => Ok(Some(PrioritizeCommand)),
             _ => Ok(None),
         }

src/handlers/relabel.rs

@@ -38,15 +38,25 @@ impl Handler for RelabelHandler {
         };
 
         if let Event::Issue(e) = event {
-            if e.action != github::IssuesAction::Opened {
-                // skip events other than opening the issue to avoid retriggering commands in the
+            if !matches!(e.action, github::IssuesAction::Opened | github::IssuesAction::Edited) {
+                // skip events other than opening or editing the issue to avoid retriggering commands in the
                 // issue body
                 return Ok(None);
             }
         }
 
         let mut input = Input::new(&body, &ctx.username);
-        match input.parse_command() {
+        let command = input.parse_command();
+        
+        if let Some(previous) = event.comment_from() {
+            let mut prev_input = Input::new(&previous, &ctx.username);
+            let prev_command = prev_input.parse_command();
+            if command == prev_command {
+                return Ok(None);
+            }
+        }
+        
+        match command {
             Command::Relabel(Ok(command)) => Ok(Some(command)),
             Command::Relabel(Err(err)) => {
                 return Err(format!(

src/lib.rs

@@ -109,6 +109,7 @@ pub async fn webhook(
                         github::IssueCommentAction::Deleted
                     }
                 },
+                changes: payload.changes,
                 issue: payload.pull_request,
                 comment: payload.review,
                 repository: payload.repository,
@@ -125,6 +126,7 @@ pub async fn webhook(
             // review comments.
             github::Event::IssueComment(github::IssueCommentEvent {
                 action: payload.action,
+                changes: payload.changes,
                 issue: payload.issue,
                 comment: payload.comment,
                 repository: payload.repository,