Re: [PATCH] interpret-trailers: add option for in-place editing

> Tobias Klauser <tobias.klauser@zhinst.com> writes:
> 
> > From: Tobias Klauser <tklauser@distanz.ch>
> >
> > Add a command line option --in-place to support in-place editing akin to
> > sed -i.  This allows to write commands like the following:
> 
> Since -i is a common shortcut for --in-place (perl -i, sed -i), it
> probably makes sense to have it here too. OTOH, this is meant for
> scripting and perhaps it's best to force script writters to be verbose.

> > Also add the corresponding documentation and tests.
> 
> This sentence does not harm, but I personnally don't think it's really
> helpfull, as it's already clear by the diffstat right below, and the
> patch itself.

> > -static void print_tok_val(const char *tok, const char *val)
> > +static void print_tok_val(FILE *fp, const char *tok, const char *val)
> >  {
> >  	char c = last_non_space_char(tok);
> >  	if (!c)
> >  		return;
> >  	if (strchr(separators, c))
> > -		printf("%s%s\n", tok, val);
> > +		fprintf(fp, "%s%s\n", tok, val);
> >  	else
> > -		printf("%s%c %s\n", tok, separators[0], val);
> > +		fprintf(fp, "%s%c %s\n", tok, separators[0], val);
> >  }
> 
> The patch would be even easier to read if split into a preparatory
> refactoring patch "turn printf into fprintf" and then the actual one.
> But it's already rather clear, so you can probably leave it as-is.

> > -static void print_lines(struct strbuf **lines, int start, int end)
> > +static void print_lines(FILE *fp, struct strbuf **lines, int start, int end)
> 
> Here and below: it would probably be more readable with a more explicit
> name for fp, like "outfile". Especially here:
> 
> > -static int process_input_file(struct strbuf **lines,
> > +static int process_input_file(FILE *fp,
> > +			      struct strbuf **lines,
> 
> Where it's tempting to think that a parameter given to
> process_input_file is ... the input file!