~aw/patches

mygit: actually use port from config v1 APPLIED

Johann Galle
Johann Galle: 6
 actually use port from config
 recognize repositories with trailing slash
 add 404 page
 add error page
 add error page
 handle nonexistent repos

 12 files changed, 145 insertions(+), 11 deletions(-)
This is an OK workaround for now, but I think there is tide middleware
that will handle this for all routes
On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote:
I wonder if there is a way to reduce code duplication here -- maybe
through middleware or something?
On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote:
Could we turn this into a general error page, making "not found" just
one instance of that erorr page? IE just take a generic error message
and use this for e.g. internal server errors as well

On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote:
Johann Galle
This is possible, but because we want flow control in this instance
(early function return), it would't really reduce duplication much.

Even when implementing it as an Err variant, we could not use the
try operator, because we would have to "convert" the Err variant to
a Ok variant. In combination with an early return, this would require
some restructuring, i.e. an if let.
Not to nitpick too much, but I have two comments:

1. I like the old school style of including the status code in the
error -- "404 -- {description}" 

2. I would make this page a bit more generic, something like:

<h1>Error</h1>

{{status_code}} -- {{description}}


Thanks for your patches!

On Sat Mar 20, 2021 at 2:24 PM PDT, Johann Galle wrote:
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~aw/patches/patches/21305/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH mygit 1/4] actually use port from config Export this patch

Johann Galle
From: Johann150 <johann@qwertqwefsday.eu>

---
 src/main.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main.rs b/src/main.rs
index 26ba66a..33d1afe 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -482,6 +482,6 @@ async fn main() -> Result<(), std::io::Error> {
    app.at("/:repo_name/tree/:ref/item/:object_name")
        .get(repo_file);
    // Raw files, patch files
    app.listen("127.0.0.1:8081").await?;
    app.listen(format!("[::]:{}", CONFIG.port)).await?;
    Ok(())
}
-- 
2.20.1
Thanks! Applied

On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote:

[PATCH mygit 2/4] recognize repositories with trailing slash Export this patch

Johann Galle
From: Johann150 <johann@qwertqwefsday.eu>

---
 src/main.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/main.rs b/src/main.rs
index 33d1afe..fe97363 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -473,6 +473,7 @@ async fn main() -> Result<(), std::io::Error> {
        .serve_file("templates/static/style.css")?; // TODO configurable
    app.at("/:repo_name").get(repo_home);
    // ALSO do git pull at this url somehow ^
    app.at("/:repo_name/").get(repo_home);
    app.at("/:repo_name/commit/:commit").get(repo_commit);
    app.at("/:repo_name/refs").get(repo_refs);
    app.at("/:repo_name/log").get(repo_log);
-- 
2.20.1
This is an OK workaround for now, but I think there is tide middleware
that will handle this for all routes

On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote:

[PATCH mygit 3/4] add 404 page Export this patch

Johann Galle
From: Johann150 <johann@qwertqwefsday.eu>

---
 src/main.rs              | 14 ++++++++++++++
 templates/not-found.html |  8 ++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 templates/not-found.html

diff --git a/src/main.rs b/src/main.rs
index fe97363..cc4b9af 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -95,6 +95,20 @@ fn args() -> Config {
    }
}

#[derive(Template)]
#[template(path = "not-found.html")]
struct NotFound<'a> {
    what: &'a str,
}

fn not_found(what: &str) -> tide::Result {
    use std::convert::TryInto;

    let body: tide::Body = NotFound { what }.try_into().unwrap();

    Ok(tide::Response::builder(404).body(body).build())
}

#[derive(Template)]
#[template(path = "index.html")] // using the template in this path, relative
struct IndexTemplate {
diff --git a/templates/not-found.html b/templates/not-found.html
new file mode 100644
index 0000000..6171e03
--- /dev/null
+++ b/templates/not-found.html
@@ -0,0 +1,8 @@
{% extends "base.html" %}

{% block content %}
  <div class="page-title"><h1>Hmm, that didn't work.</h1></div>
  <div>
    That thing you were looking for - "{{what}}" - it's not here.
  </div>
{% endblock %}
-- 
2.20.1
Could we turn this into a general error page, making "not found" just
one instance of that erorr page? IE just take a generic error message
and use this for e.g. internal server errors as well

On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote:

[PATCH mygit v2 3/4] add error page Export this patch

Johann Galle
From: Johann150 <johann@qwertqwefsday.eu>

---
v1 was: add 404 page

 src/main.rs          | 19 +++++++++++++++++++
 templates/error.html |  8 ++++++++
 2 files changed, 27 insertions(+)
 create mode 100644 templates/error.html

diff --git a/src/main.rs b/src/main.rs
index fe97363..f7cdc3b 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6,6 +6,7 @@ use git2::{
use once_cell::sync::Lazy;
use pico_args;
use serde::Deserialize;
use std::convert::TryInto;
use std::fs;
use std::path::Path;
use std::str;
@@ -95,6 +96,24 @@ fn args() -> Config {
    }
}

#[derive(Template)]
#[template(path = "error.html")]
struct ErrorTemplate<'a> {
    looking_for: &'a str,
}

fn error_page<S: TryInto<tide::StatusCode>>(status: S, looking_for: &str) -> tide::Result {
    let body: tide::Body = ErrorTemplate { looking_for }.try_into().unwrap();

    Ok(tide::Response::builder(
        status
            .try_into()
            .unwrap_or(tide::StatusCode::InternalServerError),
    )
    .body(body)
    .build())
}

#[derive(Template)]
#[template(path = "index.html")] // using the template in this path, relative
struct IndexTemplate {
diff --git a/templates/error.html b/templates/error.html
new file mode 100644
index 0000000..bf2f071
--- /dev/null
+++ b/templates/error.html
@@ -0,0 +1,8 @@
{% extends "base.html" %}

{% block content %}
  <div class="page-title"><h1>Hmm, that didn't work.</h1></div>
  <div>
    That thing you were looking for - "{{looking_for}}" - I can't give it to you. Something went wrong in getting it.
  </div>
{% endblock %}
-- 
2.20.1
Not to nitpick too much, but I have two comments:

1. I like the old school style of including the status code in the
error -- "404 -- {description}" 

2. I would make this page a bit more generic, something like:

<h1>Error</h1>

{{status_code}} -- {{description}}


Thanks for your patches!

On Sat Mar 20, 2021 at 2:24 PM PDT, Johann Galle wrote:

[PATCH mygit v3 3/4] add error page Export this patch

Johann Galle
From: Johann150 <johann@qwertqwefsday.eu>

---
v1 was: add 404 page

 Cargo.lock           |  1 +
 Cargo.toml           |  1 +
 src/errorpage.rs     | 31 +++++++++++++++++++++++++++++++
 src/main.rs          | 13 ++++++++++---
 templates/error.html |  9 +++++++++
 5 files changed, 52 insertions(+), 3 deletions(-)
 create mode 100644 src/errorpage.rs
 create mode 100644 templates/error.html

diff --git a/Cargo.lock b/Cargo.lock
index f4d2325..38149c0 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1167,6 +1167,7 @@ dependencies = [
 "askama",
 "askama_tide",
 "async-std",
 "async-trait",
 "chrono",
 "git2",
 "once_cell",
diff --git a/Cargo.toml b/Cargo.toml
index c16d847..624ebbc 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -12,6 +12,7 @@ anyhow = "1.0"
askama = {version = "0.10", features = ["with-tide"]}
askama_tide = "0.13"
async-std = { version = "1.8.0", features = ["attributes"] }
async-trait = "0.1.48"
chrono = "0.4"
git2 = {version="0.13", default-features = false}
once_cell = "1.7.2"
diff --git a/src/errorpage.rs b/src/errorpage.rs
new file mode 100644
index 0000000..dbfcba7
--- /dev/null
+++ b/src/errorpage.rs
@@ -0,0 +1,31 @@
use askama::Template;
use tide::{Middleware, Next, Request, StatusCode};

#[derive(Template)]
#[template(path = "error.html")]
struct ErrorTemplate {
    resource: String,
    status: StatusCode,
    message: String,
}

pub struct ErrorToErrorpage;

#[async_trait::async_trait]
impl<State: Clone + Send + Sync + 'static> Middleware<State> for ErrorToErrorpage{
    async fn handle(&self, req: Request<State>, next: Next<'_, State>) -> tide::Result {
        let resource = req.url().path().to_string();
        let mut response = next.run(req).await;
        if let Some(err) = response.take_error() {
            let status = err.status();
            response = ErrorTemplate{
                resource,
                status,
                message: err.into_inner().to_string(),
            }.into();
            response.set_status(status);
        }

        Ok(response)
    }
}
diff --git a/src/main.rs b/src/main.rs
index d2f390a..c1073c1 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -13,6 +13,8 @@ use syntect::highlighting::{Color, ThemeSet};
use syntect::parsing::{SyntaxReference, SyntaxSet};
use tide::Request;

mod errorpage;

#[derive(Deserialize, Debug)]
pub struct Config {
    #[serde(default = "defaults::port")]
@@ -130,14 +132,18 @@ struct RepoHomeTemplate {
    readme_text: String,
}

fn repo_from_request(repo_name: &str) -> Result<Repository> {
fn repo_from_request(repo_name: &str) -> Result<Repository, tide::Error> {
    let repo_name = percent_encoding::percent_decode_str(repo_name)
        .decode_utf8_lossy()
        .into_owned();
    let repo_path = Path::new(&CONFIG.projectroot).join(repo_name);
    // TODO CLEAN PATH! VERY IMPORTANT! DONT FORGET!
    let r = Repository::open(repo_path)?;
    Ok(r)
    Repository::open(repo_path).or_else(|_| {
        Err(tide::Error::from_str(
            404,
            "This repository does not exist.",
        ))
    })
}

async fn repo_home(req: Request<()>) -> tide::Result {
@@ -469,6 +475,7 @@ mod filters {
async fn main() -> Result<(), std::io::Error> {
    tide::log::start();
    let mut app = tide::new();
    app.with(errorpage::ErrorToErrorpage);
    app.at("/").get(index);
    app.at("/robots.txt")
        .serve_file("templates/static/robots.txt")?; // TODO configurable
diff --git a/templates/error.html b/templates/error.html
new file mode 100644
index 0000000..7a2c946
--- /dev/null
+++ b/templates/error.html
@@ -0,0 +1,9 @@
{% extends "base.html" %}

{% block content %}
  <div class="page-title"><h1>Hmm, that did not work.</h1></div>
  <div>
    Something went wrong in getting "{{resource}}":<br>
    {{status}} &mdash; {{message}}
  </div>
{% endblock %}
-- 
2.20.1

[PATCH mygit 4/4] handle nonexistent repos Export this patch

Johann Galle
From: Johann150 <johann@qwertqwefsday.eu>

---
 src/main.rs | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 7 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index cc4b9af..beeb305 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -168,7 +168,13 @@ async fn repo_home(req: Request<()>) -> tide::Result {
        Markdown,
    }

    let repo = repo_from_request(&req.param("repo_name")?)?;
    let repo = if let Ok(repo) = repo_from_request(&req.param("repo_name")?) {
        repo
    } else {
        // this repository does not exist
        return not_found(req.url().path());
    };

    if repo.is_empty().unwrap() {
        // we would not be able to find HEAD
        return Ok(RepoEmptyTemplate { repo }.into());
@@ -228,7 +234,12 @@ struct RepoLogTemplate<'a> {
}

async fn repo_log(req: Request<()>) -> tide::Result {
    let repo = repo_from_request(&req.param("repo_name")?)?;
    let repo = if let Ok(repo) = repo_from_request(&req.param("repo_name")?) {
        repo
    } else {
        // this repository does not exist
        return not_found(req.url().path());
    };
    if repo.is_empty().unwrap() {
        // redirect to start page of repo
        let mut url = req.url().clone();
@@ -276,7 +287,13 @@ struct RepoRefTemplate<'a> {
    tags: Vec<Reference<'a>>,
}
async fn repo_refs(req: Request<()>) -> tide::Result {
    let repo = repo_from_request(&req.param("repo_name")?)?;
    let repo = if let Ok(repo) = repo_from_request(&req.param("repo_name")?) {
        repo
    } else {
        // this repository does not exist
        return not_found(req.url().path());
    };

    if repo.is_empty().unwrap() {
        // redirect to start page of repo
        let mut url = req.url().clone();
@@ -311,7 +328,13 @@ struct RepoTreeTemplate<'a> {
}
async fn repo_tree(req: Request<()>) -> tide::Result {
    // TODO handle subtrees
    let repo = repo_from_request(&req.param("repo_name")?)?;
    let repo = if let Ok(repo) = repo_from_request(&req.param("repo_name")?) {
        repo
    } else {
        // this repository does not exist
        return not_found(req.url().path());
    };

    if repo.is_empty().unwrap() {
        // redirect to start page of repo
        let mut url = req.url().clone();
@@ -343,7 +366,13 @@ struct RepoCommitTemplate<'a> {
}

async fn repo_commit(req: Request<()>) -> tide::Result {
    let repo = repo_from_request(req.param("repo_name")?)?;
    let repo = if let Ok(repo) = repo_from_request(&req.param("repo_name")?) {
        repo
    } else {
        // this repository does not exist
        return not_found(req.url().path());
    };

    let commit = repo
        .revparse_single(req.param("commit")?)?
        .peel_to_commit()?;
@@ -376,8 +405,14 @@ struct RepoFileTemplate<'a> {
}

async fn repo_file(req: Request<()>) -> tide::Result {
    // TODO renmae for clarity
    let repo = repo_from_request(req.param("repo_name")?)?;
    // TODO rename for clarity
    let repo = if let Ok(repo) = repo_from_request(&req.param("repo_name")?) {
        repo
    } else {
        // this repository does not exist
        return not_found(req.url().path());
    };

    // If directory -- show tree TODO
    let head = repo.head()?;
    let spec = req.param("ref").ok().or_else(|| head.shorthand()).unwrap();
-- 
2.20.1
I wonder if there is a way to reduce code duplication here -- maybe
through middleware or something?

On Fri Mar 19, 2021 at 5:07 PM PDT, Johann Galle wrote: