[PATCH 0/3] `git format-patch --stdout id2..id`

[PATCH 0/3] `git format-patch --stdout id2..id`

[cgit]

This is a reworked version of the patch I submitted earlier.

Further comments welcome!

Lukas Fleischer (3):
  ui-patch.c: Use log_tree_commit() to generate diffs
  Allow for creating patch series
  ui-patch: Rename variables

 cmd.c      |  2 +-
 ui-patch.c | 70 ++++++++++++++++++++++++++++++++------------------------------
 ui-patch.h |  3 ++-
 3 files changed, 39 insertions(+), 36 deletions(-)

-- 
1.8.4.rc2.477.g1da3ebd

[PATCH 1/3] ui-patch.c: Use log_tree_commit() to generate diffs

[cgit]

Instead of using our own formatting, use log_tree_commit() from Git to
create patches. This removes unnecessary duplicate code and also fixes a
bug with e-mail address formatting that existed in our own
implementation.

Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
---
 ui-patch.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/ui-patch.c b/ui-patch.c
index 3922a44..429b74c 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -13,9 +13,11 @@
 
 void cgit_print_patch(char *hex, const char *prefix)
 {
+	struct rev_info rev;
 	struct commit *commit;
-	struct commitinfo *info;
 	unsigned char sha1[20], old_sha1[20];
+	char rev_range[2 * 20 + 2];
+	char *rev_argv[] = { NULL, "--reverse", rev_range };
 	char *patchname;
 
 	if (!hex)
@@ -30,36 +32,32 @@ void cgit_print_patch(char *hex, const char *prefix)
 		cgit_print_error("Bad commit reference: %s", hex);
 		return;
 	}
-	info = cgit_parse_commit(commit);
 
 	if (commit->parents && commit->parents->item)
 		hashcpy(old_sha1, commit->parents->item->object.sha1);
 	else
 		hashclr(old_sha1);
 
+	sprintf(rev_range, "%s..%s", sha1_to_hex(old_sha1), sha1_to_hex(sha1));
+
 	patchname = fmt("%s.patch", sha1_to_hex(sha1));
 	ctx.page.mimetype = "text/plain";
 	ctx.page.filename = patchname;
 	cgit_print_http_headers(&ctx);
-	htmlf("From %s Mon Sep 17 00:00:00 2001\n", sha1_to_hex(sha1));
-	htmlf("From: %s", info->author);
-	if (!ctx.cfg.noplainemail) {
-		htmlf(" %s", info->author_email);
-	}
-	html("\n");
-	html("Date: ");
-	cgit_print_date(info->author_date, "%a, %d %b %Y %H:%M:%S %z%n", ctx.cfg.local_time);
-	htmlf("Subject: %s\n\n", info->subject);
-	if (info->msg && *info->msg) {
-		htmlf("%s", info->msg);
-		if (info->msg[strlen(info->msg) - 1] != '\n')
-			html("\n");
+
+	init_revisions(&rev, NULL);
+	rev.abbrev = DEFAULT_ABBREV;
+	rev.commit_format = CMIT_FMT_EMAIL;
+	rev.verbose_header = 1;
+	rev.diff = 1;
+	rev.show_root_diff = 0;
+	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
+	setup_revisions(ARRAY_SIZE(rev_argv), (const char **)rev_argv, &rev,
+			NULL);
+	prepare_revision_walk(&rev);
+
+	while ((commit = get_revision(&rev)) != NULL) {
+		log_tree_commit(&rev, commit);
+		printf("--\ncgit %s\n", cgit_version);
 	}
-	html("---\n");
-	if (prefix)
-		htmlf("(limited to '%s')\n\n", prefix);
-	cgit_diff_tree(old_sha1, sha1, filepair_cb_raw, prefix, 0);
-	html("--\n");
-	htmlf("cgit %s\n", cgit_version);
-	cgit_free_commitinfo(info);
 }
-- 
1.8.4.rc2.477.g1da3ebd

[PATCH 2/3] Allow for creating patch series

[cgit]

This allows for specifying a revision range using the id2 parameter of
/patch/. The output that is produced is similar to

    $ git format-patch --stdout id2..id

Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
---
 cmd.c      | 2 +-
 ui-patch.c | 6 ++++--
 ui-patch.h | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/cmd.c b/cmd.c
index 9893710..0202917 100644
--- a/cmd.c
+++ b/cmd.c
@@ -98,7 +98,7 @@ static void repolist_fn(struct cgit_context *ctx)
 
 static void patch_fn(struct cgit_context *ctx)
 {
-	cgit_print_patch(ctx->qry.sha1, ctx->qry.path);
+	cgit_print_patch(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path);
 }
 
 static void plain_fn(struct cgit_context *ctx)
diff --git a/ui-patch.c b/ui-patch.c
index 429b74c..a93ba45 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -11,7 +11,7 @@
 #include "html.h"
 #include "ui-shared.h"
 
-void cgit_print_patch(char *hex, const char *prefix)
+void cgit_print_patch(char *hex, const char *old_rev, const char *prefix)
 {
 	struct rev_info rev;
 	struct commit *commit;
@@ -33,7 +33,9 @@ void cgit_print_patch(char *hex, const char *prefix)
 		return;
 	}
 
-	if (commit->parents && commit->parents->item)
+	if (old_rev)
+		get_sha1(old_rev, old_sha1);
+	else if (commit->parents && commit->parents->item)
 		hashcpy(old_sha1, commit->parents->item->object.sha1);
 	else
 		hashclr(old_sha1);
diff --git a/ui-patch.h b/ui-patch.h
index 1641cea..acd37fd 100644
--- a/ui-patch.h
+++ b/ui-patch.h
@@ -1,6 +1,6 @@
 #ifndef UI_PATCH_H
 #define UI_PATCH_H
 
-extern void cgit_print_patch(char *hex, const char *prefix);
+extern void cgit_print_patch(char *hex, const char *old_rev, const char *prefix);
 
 #endif /* UI_PATCH_H */
-- 
1.8.4.rc2.477.g1da3ebd

[PATCH 3/3] ui-patch: Rename variables

[cgit]

Rename parameters and local variables to match those from ui-diff. Also,
convert a "char *" to "const char *".

Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
---
 ui-patch.c | 28 +++++++++++++++-------------
 ui-patch.h |  3 ++-
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/ui-patch.c b/ui-patch.c
index a93ba45..3d124af 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -11,38 +11,40 @@
 #include "html.h"
 #include "ui-shared.h"
 
-void cgit_print_patch(char *hex, const char *old_rev, const char *prefix)
+void cgit_print_patch(const char *new_rev, const char *old_rev,
+		      const char *prefix)
 {
 	struct rev_info rev;
 	struct commit *commit;
-	unsigned char sha1[20], old_sha1[20];
+	unsigned char new_rev_sha1[20], old_rev_sha1[20];
 	char rev_range[2 * 20 + 2];
 	char *rev_argv[] = { NULL, "--reverse", rev_range };
 	char *patchname;
 
-	if (!hex)
-		hex = ctx.qry.head;
+	if (!new_rev)
+		new_rev = ctx.qry.head;
 
-	if (get_sha1(hex, sha1)) {
-		cgit_print_error("Bad object id: %s", hex);
+	if (get_sha1(new_rev, new_rev_sha1)) {
+		cgit_print_error("Bad object id: %s", new_rev);
 		return;
 	}
-	commit = lookup_commit_reference(sha1);
+	commit = lookup_commit_reference(new_rev_sha1);
 	if (!commit) {
-		cgit_print_error("Bad commit reference: %s", hex);
+		cgit_print_error("Bad commit reference: %s", new_rev);
 		return;
 	}
 
 	if (old_rev)
-		get_sha1(old_rev, old_sha1);
+		get_sha1(old_rev, old_rev_sha1);
 	else if (commit->parents && commit->parents->item)
-		hashcpy(old_sha1, commit->parents->item->object.sha1);
+		hashcpy(old_rev_sha1, commit->parents->item->object.sha1);
 	else
-		hashclr(old_sha1);
+		hashclr(old_rev_sha1);
 
-	sprintf(rev_range, "%s..%s", sha1_to_hex(old_sha1), sha1_to_hex(sha1));
+	sprintf(rev_range, "%s..%s", sha1_to_hex(old_rev_sha1),
+		sha1_to_hex(new_rev_sha1));
 
-	patchname = fmt("%s.patch", sha1_to_hex(sha1));
+	patchname = fmt("%s.patch", sha1_to_hex(new_rev_sha1));
 	ctx.page.mimetype = "text/plain";
 	ctx.page.filename = patchname;
 	cgit_print_http_headers(&ctx);
diff --git a/ui-patch.h b/ui-patch.h
index acd37fd..7a6cacd 100644
--- a/ui-patch.h
+++ b/ui-patch.h
@@ -1,6 +1,7 @@
 #ifndef UI_PATCH_H
 #define UI_PATCH_H
 
-extern void cgit_print_patch(char *hex, const char *old_rev, const char *prefix);
+extern void cgit_print_patch(const char *new_rev, const char *old_rev,
+			     const char *prefix);
 
 #endif /* UI_PATCH_H */
-- 
1.8.4.rc2.477.g1da3ebd

[PATCH 1/3] ui-patch.c: Use log_tree_commit() to generate diffs

[Jason]

On Thu, Aug 15, 2013 at 11:06 AM, Lukas Fleischer <cgit at cryptocrack.de> wrote:
> +       char rev_range[2 * 20 + 2];>
> +       sprintf(rev_range, "%s..%s", sha1_to_hex(old_sha1), sha1_to_hex(sha1));

If the strings coming out of sha1_to_hex were of length twenty, then
rev_range would have to be of size 2*20+3, not +2, to account for the
null terminator. But in fact sha1_to_hex produces length forty
strings, so rev_range actually needs to be 2*40+3.

        for (i = 0; i < 20; i++) {
                unsigned int val = *sha1++;
                *buf++ = hex[val >> 4];
                *buf++ = hex[val & 0xf];
        }

[PATCH 1/3] ui-patch.c: Use log_tree_commit() to generate diffs

[cgit]

On Thu, Aug 15, 2013 at 12:37:03PM -0600, Jason A. Donenfeld wrote:
> On Thu, Aug 15, 2013 at 11:06 AM, Lukas Fleischer <cgit at cryptocrack.de> wrote:
> > +       char rev_range[2 * 20 + 2];>
> > +       sprintf(rev_range, "%s..%s", sha1_to_hex(old_sha1), sha1_to_hex(sha1));
> 
> If the strings coming out of sha1_to_hex were of length twenty, then
> rev_range would have to be of size 2*20+3, not +2, to account for the
> null terminator. But in fact sha1_to_hex produces length forty
> strings, so rev_range actually needs to be 2*40+3.

Ugh. I did this right in the original patch (well, at least the null
character) -- no idea why I messed it up here. I probably need more
coffee :)

Fixed it locally. Thanks! I will wait for further comments and resubmit
in a couple of hours.

> 
>         for (i = 0; i < 20; i++) {
>                 unsigned int val = *sha1++;
>                 *buf++ = hex[val >> 4];
>                 *buf++ = hex[val & 0xf];
>         }

[PATCH 2/3] Allow for creating patch series

[john]

On Thu, Aug 15, 2013 at 07:06:35PM +0200, Lukas Fleischer wrote:
> This allows for specifying a revision range using the id2 parameter of
> /patch/. The output that is produced is similar to
> 
>     $ git format-patch --stdout id2..id
> 
> Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> ---
>  cmd.c      | 2 +-
>  ui-patch.c | 6 ++++--
>  ui-patch.h | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd.c b/cmd.c
> index 9893710..0202917 100644
> --- a/cmd.c
> +++ b/cmd.c
> @@ -98,7 +98,7 @@ static void repolist_fn(struct cgit_context *ctx)
>  
>  static void patch_fn(struct cgit_context *ctx)
>  {
> -	cgit_print_patch(ctx->qry.sha1, ctx->qry.path);
> +	cgit_print_patch(ctx->qry.sha1, ctx->qry.sha2, ctx->qry.path);
>  }
>  
>  static void plain_fn(struct cgit_context *ctx)
> diff --git a/ui-patch.c b/ui-patch.c
> index 429b74c..a93ba45 100644
> --- a/ui-patch.c
> +++ b/ui-patch.c
> @@ -11,7 +11,7 @@
>  #include "html.h"
>  #include "ui-shared.h"
>  
> -void cgit_print_patch(char *hex, const char *prefix)
> +void cgit_print_patch(char *hex, const char *old_rev, const char *prefix)
>  {
>  	struct rev_info rev;
>  	struct commit *commit;
> @@ -33,7 +33,9 @@ void cgit_print_patch(char *hex, const char *prefix)
>  		return;
>  	}
>  
> -	if (commit->parents && commit->parents->item)
> +	if (old_rev)
> +		get_sha1(old_rev, old_sha1);

Should we check the return value of get_sha1() here?  A few lines above
we print an error message if 'hex' isn't found.

> +	else if (commit->parents && commit->parents->item)
>  		hashcpy(old_sha1, commit->parents->item->object.sha1);
>  	else
>  		hashclr(old_sha1);
> diff --git a/ui-patch.h b/ui-patch.h
> index 1641cea..acd37fd 100644
> --- a/ui-patch.h
> +++ b/ui-patch.h
> @@ -1,6 +1,6 @@
>  #ifndef UI_PATCH_H
>  #define UI_PATCH_H
>  
> -extern void cgit_print_patch(char *hex, const char *prefix);
> +extern void cgit_print_patch(char *hex, const char *old_rev, const char *prefix);
>  
>  #endif /* UI_PATCH_H */
> -- 
> 1.8.4.rc2.477.g1da3ebd