split off diff action from log action

2 files changed, 89 insertions, 77 deletions

diff --git a/src/get_routes.rs b/src/get_routes.rs
index effc68a..b209cab 100644
--- a/src/get_routes.rs
+++ b/src/get_routes.rs
@@ -34,6 +34,7 @@ pub(crate) fn get_blob<C: Controller>(
         "edit" => edit_blob(entr, params, controller, ctx),
         "upload" => Ok(forms::upload_form(true, controller, &ctx).into()),
         "log" => log_blob(entr, params, controller, ctx),
+        "diff" => diff_blob(entr, params, controller, ctx),
         "raw" => raw_blob(entr, params, controller, ctx),
         "move" => move_blob(entr, params, controller, ctx),
         "remove" => remove_blob(entr, params, controller, ctx),
@@ -118,90 +119,14 @@ fn edit_blob<C: Controller>(
     }
 }
 
-#[derive(Deserialize)]
-struct LogParam {
-    commit: Option<String>,
-}
-
 fn log_blob<C: Controller>(
     _entr: TreeEntry,
     params: ActionParam,
     controller: &C,
     ctx: Context,
 ) -> Result<Response, Error> {
-    let log_param: LogParam = ctx.parts.query().unwrap();
-
     let filename = ctx.file_name().unwrap();
 
-    // TODO: move to separate function
-    if let Some(commit) = log_param.commit {
-        let branch_commit = ctx.branch_head()?;
-
-        let commit = ctx
-            .repo
-            .find_commit(Oid::from_str(&commit)?)
-            .map_err(|_| Error::NotFound("commit not found".into()))?;
-
-        if branch_commit.id() != commit.id()
-            && !ctx
-                .repo
-                .graph_descendant_of(branch_commit.id(), commit.id())?
-        {
-            // disallow viewing commits from other branches you shouldn't have access to
-            return Err(Error::NotFound("commit not found".into()));
-        }
-
-        let blob_id = if let Ok(entry) = commit.tree()?.get_path(&ctx.path) {
-            entry.id()
-        } else {
-            return Ok(Page {
-                title: format!("Commit for {}", filename),
-                body: "file removed".into(),
-                header: Some(action_links(&params.action, controller, &ctx)),
-                controller,
-                parts: &ctx.parts,
-            }
-            .into());
-        };
-
-        // TODO: if UTF-8 decoding fails, link ?action=raw&rev=
-        // TODO: what if there are multiple parents?
-        let old_blob_id = commit
-            .parents()
-            .next()
-            .and_then(|p| p.tree().unwrap().get_path(&ctx.path).ok())
-            .map(|e| e.id());
-        if Some(blob_id) != old_blob_id {
-            let mut page = Page {
-                title: format!("Commit for {}", filename),
-                header: Some(action_links(&params.action, controller, &ctx)),
-                body: format!(
-                    "<h1>{}</h1>{} committed on {}",
-                    html_escape(commit.summary().unwrap_or_default()),
-                    html_escape(commit.author().name().unwrap_or_default()),
-                    NaiveDateTime::from_timestamp(commit.time().seconds(), 0)
-                        .format("%b %d, %Y, %H:%M")
-                ),
-                controller,
-                parts: &ctx.parts,
-            };
-            if let Some(old_blob_id) = old_blob_id {
-                page.body.push_str(&diff(
-                    from_utf8(ctx.repo.find_blob(old_blob_id)?.content())?,
-                    from_utf8(ctx.repo.find_blob(blob_id)?.content())?,
-                ))
-            } else {
-                page.body.push_str(&diff(
-                    "",
-                    from_utf8(&ctx.repo.find_blob(blob_id)?.content())?,
-                ));
-            }
-            return Ok(page.into());
-        } else {
-            return Err(Error::NotFound("commit not found".into()));
-        }
-    }
-
     let mut page = Page {
         title: format!("Log for {}", filename),
         body: String::new(),
@@ -253,7 +178,7 @@ fn log_blob<C: Controller>(
         }
 
         page.body.push_str(&format!(
-            "<li><a href='?action=log&commit={}'>{}: {}</a></li>",
+            "<li><a href='?action=diff&id={}'>{}: {}</a></li>",
             html_escape(c.id().to_string()),
             html_escape(c.author().name().unwrap_or_default()),
             html_escape(c.summary().unwrap_or_default()),
@@ -264,6 +189,86 @@ fn log_blob<C: Controller>(
     Ok(page.into())
 }
 
+#[derive(Deserialize)]
+struct DiffParam {
+    id: String,
+}
+
+fn diff_blob<C: Controller>(
+    _entr: TreeEntry,
+    params: ActionParam,
+    controller: &C,
+    ctx: Context,
+) -> Result<Response, Error> {
+    let param: DiffParam = ctx.parts.query()?;
+    let branch_commit = ctx.branch_head()?;
+
+    let commit = ctx
+        .repo
+        .find_commit(Oid::from_str(&param.id)?)
+        .map_err(|_| Error::NotFound("commit not found".into()))?;
+
+    if branch_commit.id() != commit.id()
+        && !ctx
+            .repo
+            .graph_descendant_of(branch_commit.id(), commit.id())?
+    {
+        // disallow viewing commits from other branches you shouldn't have access to
+        return Err(Error::NotFound("commit not found".into()));
+    }
+
+    let blob_id = if let Ok(entry) = commit.tree()?.get_path(&ctx.path) {
+        entry.id()
+    } else {
+        return Ok(Page {
+            title: format!("Commit for {}", ctx.file_name().unwrap()),
+            body: "file removed".into(),
+            header: Some(action_links(&params.action, controller, &ctx)),
+            controller,
+            parts: &ctx.parts,
+        }
+        .into());
+    };
+
+    // TODO: if UTF-8 decoding fails, link ?action=raw&rev=
+    // TODO: what if there are multiple parents?
+    let old_blob_id = commit
+        .parents()
+        .next()
+        .and_then(|p| p.tree().unwrap().get_path(&ctx.path).ok())
+        .map(|e| e.id());
+
+    if Some(blob_id) != old_blob_id {
+        let mut page = Page {
+            title: format!("Commit for {}", ctx.file_name().unwrap()),
+            header: Some(action_links(&params.action, controller, &ctx)),
+            body: format!(
+                "<h1>{}</h1>{} committed on {}",
+                html_escape(commit.summary().unwrap_or_default()),
+                html_escape(commit.author().name().unwrap_or_default()),
+                NaiveDateTime::from_timestamp(commit.time().seconds(), 0)
+                    .format("%b %d, %Y, %H:%M")
+            ),
+            controller,
+            parts: &ctx.parts,
+        };
+        if let Some(old_blob_id) = old_blob_id {
+            page.body.push_str(&diff(
+                from_utf8(ctx.repo.find_blob(old_blob_id)?.content())?,
+                from_utf8(ctx.repo.find_blob(blob_id)?.content())?,
+            ))
+        } else {
+            page.body.push_str(&diff(
+                "",
+                from_utf8(&ctx.repo.find_blob(blob_id)?.content())?,
+            ));
+        }
+        Ok(page.into())
+    } else {
+        Err(Error::NotFound("commit not found".into()))
+    }
+}
+
 fn raw_blob<C: Controller>(
     entr: TreeEntry,
     _params: ActionParam,
diff --git a/src/main.rs b/src/main.rs
index fc0b7ce..658a245 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -21,6 +21,7 @@ use serde::Deserialize;
 use sputnik::html_escape;
 use sputnik::hyper_body::FormError;
 use sputnik::mime;
+use sputnik::request::QueryError;
 use sputnik::request::SputnikParts;
 use sputnik::response::SputnikBuilder;
 use std::convert::Infallible;
@@ -496,6 +497,12 @@ impl From<FormError> for Error {
     }
 }
 
+impl From<QueryError> for Error {
+    fn from(e: QueryError) -> Self {
+        Self::BadRequest(e.to_string())
+    }
+}
+
 pub struct Context {
     repo: Repository,
     parts: Parts,