Improve handling of skip chapter, hard reset (#13)

Makefile.toml

@@ -3,7 +3,7 @@ skip_core_tasks = true
 default_to_workspace = false
 
 [tasks.bundle]
-script = "dx bundle"
+script = "dx bundle --release"
 
 [tasks.watch]
 script = "cargo watch -s 'dx build'"

assets/main.css

@@ -130,7 +130,7 @@ input[type=file] {
   display: none;
 }
 
-button, label {
+button, label, input[type=submit] {
   display: inline-block;
   appearance: none;
   background: #eee;
@@ -206,4 +206,16 @@ button, label {
   select {
     max-width: 100%;
   }
+}
+
+dialog {
+  z-index: 1000; 
+  max-width: 400px;
+  box-shadow: 4px 4px 8px #aaa;
+
+  form {
+    margin-top: 1rem;
+    display: flex;
+    gap: 0.5rem;
+  }
 }

src/git.rs

@@ -40,6 +40,11 @@ pub struct GitRepo {}
 
 pub const UPSTREAM: &str = "upstream";
 
+pub enum MergeType {
+  CherryPick,
+  HardReset,
+}
+
 impl GitRepo {
   pub fn new() -> Self {
     GitRepo {}
@@ -66,7 +71,11 @@ impl GitRepo {
     Ok(())
   }
 
-  pub fn create_branch_from(&self, target_branch: &str, base_branch: &str) -> Result<String> {
+  pub fn create_branch_from(
+    &self,
+    target_branch: &str,
+    base_branch: &str,
+  ) -> Result<(String, MergeType)> {
     git(|cmd| {
       cmd.args(["checkout", "-b", target_branch]);
     })
@@ -79,30 +88,35 @@ impl GitRepo {
       ]);
     });
 
-    if res.is_err() {
-      tracing::warn!("Merge conflicts when cherry-picking, resorting to hard reset");
+    let merge_type = match res {
+      Ok(_) => MergeType::CherryPick,
+      Err(e) => {
+        tracing::warn!("Merge conflicts when cherry-picking, resorting to hard reset: ${e:?}");
+
+        git(|cmd| {
+          cmd.args(["cherry-pick", "--abort"]);
+        })
+        .context("Failed to abort cherry-pick")?;
 
-      git(|cmd| {
-        cmd.args(["cherry-pick", "--abort"]);
-      })
-      .context("Failed to abort cherry-pick")?;
+        let upstream_target = format!("{UPSTREAM}/{target_branch}");
+        git(|cmd| {
+          cmd.args(["reset", "--hard", &upstream_target]);
+        })
+        .with_context(|| format!("Failed to hard reset to {upstream_target}"))?;
 
-      let upstream_target = format!("{UPSTREAM}/{target_branch}");
-      git(|cmd| {
-        cmd.args(["reset", "--hard", &upstream_target]);
-      })
-      .with_context(|| format!("Failed to hard reset to {upstream_target}"))?;
+        git(|cmd| {
+          cmd.args(["reset", "--soft", "main"]);
+        })
+        .context("Failed to soft reset to main")?;
 
-      git(|cmd| {
-        cmd.args(["reset", "--soft", "main"]);
-      })
-      .context("Failed to soft reset to main")?;
+        git(|cmd| {
+          cmd.args(["commit", "-m", "Override with reference solution"]);
+        })
+        .context("Failed to commit reference solution")?;
 
-      git(|cmd| {
-        cmd.args(["commit", "-m", "Override with reference solution"]);
-      })
-      .context("Failed to commit reference solution")?;
-    }
+        MergeType::HardReset
+      }
+    };
 
     git(|cmd| {
       cmd.args(["push", "-u", "origin", target_branch]);
@@ -116,7 +130,7 @@ impl GitRepo {
     })
     .context("Failed to checkout main")?;
 
-    Ok(head)
+    Ok((head, merge_type))
   }
 
   pub fn checkout_main_and_pull(&self) -> Result<()> {

src/github.rs

@@ -22,7 +22,7 @@ use std::{env, fs, process::Command, sync::Arc, time::Duration};
 use tokio::{time::timeout, try_join};
 use tracing::warn;
 
-use crate::utils;
+use crate::{git::MergeType, utils};
 
 pub struct GithubRepo {
   user: String,
@@ -37,6 +37,8 @@ pub enum PullSelector {
   Label(String),
 }
 
+const RESET_LABEL: &str = "reset";
+
 impl GithubRepo {
   pub fn new(user: &str, name: &str) -> Self {
     GithubRepo {
@@ -216,8 +218,22 @@ impl GithubRepo {
     base: &GithubRepo,
     base_pr: &PullRequest,
     head: &str,
+    merge_type: MergeType,
   ) -> Result<PullRequest> {
     let pulls = self.pr_handler();
+    let mut body = base_pr
+      .body
+      .as_ref()
+      .expect("Author error: PR missing body")
+      .clone();
+
+    let is_reset = matches!(merge_type, MergeType::HardReset);
+    if is_reset {
+      body.push_str(r#"
+      
+Note: due to a merge conflict, this PR is a hard reset to the reference solution, and may have overwritten your previous changes."#);
+    }
+
     let request = pulls
       .create(
         base_pr
@@ -227,23 +243,21 @@ impl GithubRepo {
         &base_pr.head.ref_field,
         "main", // don't copy base
       )
-      .body(
-        base_pr
-          .body
-          .as_ref()
-          .expect("Author error: PR missing body"),
-      );
+      .body(body);
     let self_pr = request.send().await?;
 
     // TODO: lots of parallelism below we should exploit
 
-    let labels = match &base_pr.labels {
+    let mut labels = match &base_pr.labels {
       Some(labels) => labels
         .iter()
         .map(|label| label.name.clone())
         .collect::<Vec<_>>(),
       None => Vec::new(),
     };
+    if is_reset {
+      labels.push(RESET_LABEL.into());
+    }
     self
       .issue_handler()
       .add_labels(self_pr.number, &labels)

src/quest.rs

@@ -51,10 +51,13 @@ impl QuestConfig {
 }
 
 #[derive(Clone, Debug)]
-pub struct QuestState {
-  pub stage: Stage,
-  pub part: StagePart,
-  pub status: StagePartStatus,
+pub enum QuestState {
+  Ongoing {
+    stage: Stage,
+    part: StagePart,
+    status: StagePartStatus,
+  },
+  Completed,
 }
 
 pub struct Quest {
@@ -174,7 +177,7 @@ impl Quest {
         },
         ..
       }) => {
-        return Ok(QuestState {
+        return Ok(QuestState::Ongoing {
           stage: self.stages[0].clone(),
           part: StagePart::Starter,
           status: StagePartStatus::Start,
@@ -227,14 +230,14 @@ impl Quest {
       })
     });
 
-    tracing::debug!("PRs: {:#?}", pr_stages.clone().collect::<Vec<_>>());
-    tracing::debug!("Issues: {:#?}", issue_stages.clone().collect::<Vec<_>>());
+    tracing::trace!("PRs: {:#?}", pr_stages.clone().collect::<Vec<_>>());
+    tracing::trace!("Issues: {:#?}", issue_stages.clone().collect::<Vec<_>>());
 
     let Some((stage, part, finished)) = pr_stages
       .chain(issue_stages)
       .max_by_key(|(stage, part, finished)| (stage.idx, *part, *finished))
     else {
-      return Ok(QuestState {
+      return Ok(QuestState::Ongoing {
         stage: self.stages[0].clone(),
         part: StagePart::Starter,
         status: StagePartStatus::Start,
@@ -243,19 +246,25 @@ impl Quest {
 
     Ok(if finished {
       match part.next_part() {
-        Some(next_part) => QuestState {
+        Some(next_part) => QuestState::Ongoing {
           stage,
           part: next_part,
           status: StagePartStatus::Start,
         },
-        None => QuestState {
-          stage: self.stages[stage.idx + 1].clone(),
-          part: StagePart::Starter,
-          status: StagePartStatus::Start,
-        },
+        None => {
+          if stage.idx == self.stages.len() - 1 {
+            QuestState::Completed
+          } else {
+            QuestState::Ongoing {
+              stage: self.stages[stage.idx + 1].clone(),
+              part: StagePart::Starter,
+              status: StagePartStatus::Start,
+            }
+          }
+        }
       }
     } else {
-      QuestState {
+      QuestState::Ongoing {
         stage,
         part,
         status: StagePartStatus::Ongoing,
@@ -303,7 +312,7 @@ impl Quest {
   async fn file_pr(&self, target_branch: &str, base_branch: &str) -> Result<PullRequest> {
     self.origin_git.checkout_main_and_pull()?;
 
-    let branch_head = self
+    let (branch_head, merge_type) = self
       .origin_git
       .create_branch_from(target_branch, base_branch)?;
 
@@ -314,7 +323,7 @@ impl Quest {
       .clone();
     let new_pr = self
       .origin
-      .copy_pr(&self.upstream, &pr, &branch_head)
+      .copy_pr(&self.upstream, &pr, &branch_head, merge_type)
       .await?;
 
     tracing::debug!("Filed PR: {base_branch} -> {target_branch}");
@@ -524,7 +533,14 @@ mod test {
     macro_rules! state_is {
       ($a:expr, $b:expr, $c:expr) => {
         let state = quest.infer_state().await?;
-        assert_eq!((state.stage.idx, state.part, state.status), ($a, $b, $c));
+        match state {
+          QuestState::Ongoing {
+            stage,
+            part,
+            status,
+          } => assert_eq!((stage.idx, part, status), ($a, $b, $c)),
+          QuestState::Completed => panic!("finished"),
+        };
       };
     }
 
@@ -564,7 +580,14 @@ mod test {
     macro_rules! state_is {
       ($a:expr, $b:expr, $c:expr) => {
         let state = quest.infer_state().await?;
-        assert_eq!((state.stage.idx, state.part, state.status), ($a, $b, $c));
+        match state {
+          QuestState::Ongoing {
+            stage,
+            part,
+            status,
+          } => assert_eq!((stage.idx, part, status), ($a, $b, $c)),
+          QuestState::Completed => panic!("finished"),
+        };
       };
     }