Re: [RFC PATCH 1/5] format-patch: allow additional generated content in make_cover_letter()
On Tue, Jul 17, 2018 at 6:15 AM Johannes Schindelin wrote: > On Wed, 30 May 2018, Eric Sunshine wrote: > > make_cover_letter() returns early when it lacks sufficient state to emit > > a diffstat, which makes it difficult to extend the function to reliably > > emit additional generated content. Work around this shortcoming by > > factoring diffstat-printing logic out to its own function and calling it > > as needed without otherwise inhibiting normal control flow. > > > > Signed-off-by: Eric Sunshine > > Makes sense. Thanks, but it's probably not worth spending time reviewing this RFC series. I already have a new series in the works (in fact, mostly finished) in which the implementation is drastically changed from this one. Aside from adding an --interdiff option to git-format-patch (in addition to a --range-diff option) and allowing interdiff and range-diff to be added as commentary to a single-patch, the new series also takes advantage of the newly-libified range-diff engine rather than running git-range-diff as a command. So, most or all of the code has changed. (Though, perhaps it wouldn't hurt to review the documentation changes in this RFC series to see if I botched how I described the option.)
Re: [RFC PATCH 1/5] format-patch: allow additional generated content in make_cover_letter()
Hi Eric,
On Wed, 30 May 2018, Eric Sunshine wrote:
> make_cover_letter() returns early when it lacks sufficient state to emit
> a diffstat, which makes it difficult to extend the function to reliably
> emit additional generated content. Work around this shortcoming by
> factoring diffstat-printing logic out to its own function and calling it
> as needed without otherwise inhibiting normal control flow.
>
> Signed-off-by: Eric Sunshine
Makes sense.
Ciao,
Dscho
> ---
> builtin/log.c | 43 +++
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 71f68a3e4f..e01a256c11 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -992,6 +992,26 @@ static char *find_branch_name(struct rev_info *rev)
> return branch;
> }
>
> +static void emit_diffstat(struct rev_info *rev,
> + struct commit *origin, struct commit *head)
> +{
> + struct diff_options opts;
> +
> + memcpy(, >diffopt, sizeof(opts));
> + opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> + opts.stat_width = MAIL_DEFAULT_WRAP;
> +
> + diff_setup_done();
> +
> + diff_tree_oid(>tree->object.oid,
> + >tree->object.oid,
> + "", );
> + diffcore_std();
> + diff_flush();
> +
> + fprintf(rev->diffopt.file, "\n");
> +}
> +
> static void make_cover_letter(struct rev_info *rev, int use_stdout,
> struct commit *origin,
> int nr, struct commit **list,
> @@ -1005,7 +1025,6 @@ static void make_cover_letter(struct rev_info *rev, int
> use_stdout,
> struct strbuf sb = STRBUF_INIT;
> int i;
> const char *encoding = "UTF-8";
> - struct diff_options opts;
> int need_8bit_cte = 0;
> struct pretty_print_context pp = {0};
> struct commit *head = list[0];
> @@ -1055,25 +1074,9 @@ static void make_cover_letter(struct rev_info *rev,
> int use_stdout,
>
> shortlog_output();
>
> - /*
> - * We can only do diffstat with a unique reference point
> - */
> - if (!origin)
> - return;
> -
> - memcpy(, >diffopt, sizeof(opts));
> - opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
> - opts.stat_width = MAIL_DEFAULT_WRAP;
> -
> - diff_setup_done();
> -
> - diff_tree_oid(>tree->object.oid,
> - >tree->object.oid,
> - "", );
> - diffcore_std();
> - diff_flush();
> -
> - fprintf(rev->diffopt.file, "\n");
> + /* We can only do diffstat with a unique reference point */
> + if (origin)
> + emit_diffstat(rev, origin, head);
> }
>
> static const char *clean_message_id(const char *msg_id)
> --
> 2.17.1.1235.ge295dfb56e
>
>
[RFC PATCH 1/5] format-patch: allow additional generated content in make_cover_letter()
make_cover_letter() returns early when it lacks sufficient state to emit
a diffstat, which makes it difficult to extend the function to reliably
emit additional generated content. Work around this shortcoming by
factoring diffstat-printing logic out to its own function and calling it
as needed without otherwise inhibiting normal control flow.
Signed-off-by: Eric Sunshine
---
builtin/log.c | 43 +++
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 71f68a3e4f..e01a256c11 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -992,6 +992,26 @@ static char *find_branch_name(struct rev_info *rev)
return branch;
}
+static void emit_diffstat(struct rev_info *rev,
+ struct commit *origin, struct commit *head)
+{
+ struct diff_options opts;
+
+ memcpy(, >diffopt, sizeof(opts));
+ opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
+ opts.stat_width = MAIL_DEFAULT_WRAP;
+
+ diff_setup_done();
+
+ diff_tree_oid(>tree->object.oid,
+ >tree->object.oid,
+ "", );
+ diffcore_std();
+ diff_flush();
+
+ fprintf(rev->diffopt.file, "\n");
+}
+
static void make_cover_letter(struct rev_info *rev, int use_stdout,
struct commit *origin,
int nr, struct commit **list,
@@ -1005,7 +1025,6 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
struct strbuf sb = STRBUF_INIT;
int i;
const char *encoding = "UTF-8";
- struct diff_options opts;
int need_8bit_cte = 0;
struct pretty_print_context pp = {0};
struct commit *head = list[0];
@@ -1055,25 +1074,9 @@ static void make_cover_letter(struct rev_info *rev, int
use_stdout,
shortlog_output();
- /*
-* We can only do diffstat with a unique reference point
-*/
- if (!origin)
- return;
-
- memcpy(, >diffopt, sizeof(opts));
- opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
- opts.stat_width = MAIL_DEFAULT_WRAP;
-
- diff_setup_done();
-
- diff_tree_oid(>tree->object.oid,
- >tree->object.oid,
- "", );
- diffcore_std();
- diff_flush();
-
- fprintf(rev->diffopt.file, "\n");
+ /* We can only do diffstat with a unique reference point */
+ if (origin)
+ emit_diffstat(rev, origin, head);
}
static const char *clean_message_id(const char *msg_id)
--
2.17.1.1235.ge295dfb56e
