Re: [PATCH 2/4] builtin/interpret-trailers: suppress blank line

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Apr 07, 2016 at 10:00:37AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > it's sometimes useful to be able to pass output message of
> > git-mailinfo through git-interpret-trailers,
> > but that creates problems since that does not
> > include the subject and an empty line after that,
> > making interpret-trailers add an empty line.
> >
> > Add a flag to bypass adding the blank line.
> 
> I think I understand what you are trying to do, but using output
> that comes from 'mailinfo' alone as the input to anything (including
> interpret-trailers) does not make much sense.
> 
> If you use the mailinfo output in the way it is expected to be used,
> i.e. take the subject from the "info" that goes to its standard
> output and append the "msg" with a blank between them, and feed the
> result to interpret-trailers, do you still need this step in your
> series?

No - but then I will need to re-run mailinfo to parse the result,
will I not?

And unfortunately it appears that interpret-trailers can't
handle arbitrary mail - it wants data from commit.

> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  trailer.h                    |  2 +-
> >  builtin/interpret-trailers.c |  9 +++++++--
> >  trailer.c                    | 10 +++++++---
> >  3 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/trailer.h b/trailer.h
> > index 36b40b8..afcf680 100644
> > --- a/trailer.h
> > +++ b/trailer.h
> > @@ -2,6 +2,6 @@
> >  #define TRAILER_H
> >  
> >  void process_trailers(const char *file, int in_place, int trim_empty,
> > -		      struct string_list *trailers);
> > +		      int suppress_blank_line, struct string_list *trailers);
> >  
> >  #endif /* TRAILER_H */
> > diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
> > index 18cf640..4a92788 100644
> > --- a/builtin/interpret-trailers.c
> > +++ b/builtin/interpret-trailers.c
> > @@ -18,11 +18,14 @@ static const char * const git_interpret_trailers_usage[] = {
> >  
> >  int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
> >  {
> > +	int suppress_blank_line = 0;
> >  	int in_place = 0;
> >  	int trim_empty = 0;
> >  	struct string_list trailers = STRING_LIST_INIT_DUP;
> >  
> >  	struct option options[] = {
> > +		OPT_BOOL(0, "suppress-blank-line", &suppress_blank_line,
> > +			 N_("suppress prefixing tailer(s) with a blank line ")),
> >  		OPT_BOOL(0, "in-place", &in_place, N_("edit files in place")),
> >  		OPT_BOOL(0, "trim-empty", &trim_empty, N_("trim empty trailers")),
> >  		OPT_STRING_LIST('t', "trailer", &trailers, N_("trailer"),
> > @@ -36,11 +39,13 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
> >  	if (argc) {
> >  		int i;
> >  		for (i = 0; i < argc; i++)
> > -			process_trailers(argv[i], in_place, trim_empty, &trailers);
> > +			process_trailers(argv[i], in_place, trim_empty,
> > +					 suppress_blank_line, &trailers);
> >  	} else {
> >  		if (in_place)
> >  			die(_("no input file given for in-place editing"));
> > -		process_trailers(NULL, in_place, trim_empty, &trailers);
> > +		process_trailers(NULL, in_place, trim_empty,
> > +				 suppress_blank_line, &trailers);
> >  	}
> >  
> >  	string_list_clear(&trailers, 0);
> > diff --git a/trailer.c b/trailer.c
> > index 8e48a5c..8e5be91 100644
> > --- a/trailer.c
> > +++ b/trailer.c
> > @@ -805,6 +805,7 @@ static void print_lines(FILE *outfile, struct strbuf **lines, int start, int end
> >  
> >  static int process_input_file(FILE *outfile,
> >  			      struct strbuf **lines,
> > +			      int suppress_blank_line,
> >  			      struct trailer_item **in_tok_first,
> >  			      struct trailer_item **in_tok_last)
> >  {
> > @@ -822,7 +823,8 @@ static int process_input_file(FILE *outfile,
> >  	/* Print lines before the trailers as is */
> >  	print_lines(outfile, lines, 0, trailer_start);
> >  
> > -	if (!has_blank_line_before(lines, trailer_start - 1))
> > +	if (!suppress_blank_line &&
> > +	    !has_blank_line_before(lines, trailer_start - 1))
> >  		fprintf(outfile, "\n");
> >  
> >  	/* Parse trailer lines */
> > @@ -875,7 +877,8 @@ static FILE *create_in_place_tempfile(const char *file)
> >  	return outfile;
> >  }
> >  
> > -void process_trailers(const char *file, int in_place, int trim_empty, struct string_list *trailers)
> > +void process_trailers(const char *file, int in_place, int trim_empty,
> > +		      int suppress_blank_line, struct string_list *trailers)
> >  {
> >  	struct trailer_item *in_tok_first = NULL;
> >  	struct trailer_item *in_tok_last = NULL;
> > @@ -894,7 +897,8 @@ void process_trailers(const char *file, int in_place, int trim_empty, struct str
> >  		outfile = create_in_place_tempfile(file);
> >  
> >  	/* Print the lines before the trailers */
> > -	trailer_end = process_input_file(outfile, lines, &in_tok_first, &in_tok_last);
> > +	trailer_end = process_input_file(outfile, lines, suppress_blank_line,
> > +					 &in_tok_first, &in_tok_last);
> >  
> >  	arg_tok_first = process_command_line_args(trailers);