Re: [PATCH v2 2/4] Add format.coverauto boolean

[Frank Terbeck <ft@bewatermyfriend.org>]

Stephen Boyd <bebarino@gmail.com>:
> On Mon, May 4, 2009 at 2:59 AM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> > An exception is if it is called using the --stdout option,
> > which disables format.coverauto, because users of --stdout
> > (like git-rebase.sh) usually are not interested in
> > cover letters at all.
> >
> 
> Would it make more sense to just have git-rebase.sh use
> --cover-letter=never? I thought configuration variables were defaults
> which have to be overridden.

Could be done. And in an earlier version I did that. Since that took
changes in more places, I reverted to this less intrusive approach.

Also, this will keep the scripts of people who use --stdout working,
no matter what the settings of an individual user might be.

I don't think people who use --stdout will want the cover letter
(which is always the same) in the output. Since you can still force
its output, I think this would be reasonable compromise.

> Also, why does this variable even exist? I think Jeff's suggestion is
> best, where you can set format.coverletter to always, never, or some
> number.

Well, I did this because I wanted both Junio's and Jeff's suggestions
to be incorporated.

Junio correctly stated, that nobody will usually want cover letters
for one-patch "series". Which I think is right; and the wrapper I used
for this before did take that into account as well. So coverletter had
to default to 2.

If that where to enable the generation of cover letters for all
format-patch calls with every patch series that is at least two
patches long, it would change format-patch's default behaviour;
potentially breaking people's scripts (similarly to the way
coverauto=true could break git-rebase.sh without either adding
--cover-letter=never or the exception in the --stdout codepath).

That's why I think this should be handled in two separate options.
And as I mentioned, coverauto could be left out if we'd advise users
who want that to create an git alias that does 'format-patch
--cover-letter'.

> > +	if (!strcmp(var, "format.coverauto")) {
> > +		cover_letter = git_config_bool(var, value);;
> > +		return 0;
> > +	}
> 
> Double semi-colon?

Oops, yeah. I'll resend this one (if coverauto turns out to be the way
to go).

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925