Sorry for taking this long, I was covered in work.
Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Frank Terbeck <ft@xxxxxxxxxxxxxxxxxxx> writes:
> > [format]
> > coverletter = true
> > coveronepatch = false
>
> Nobody wants a cover letter to a single patch, so a better way
> would probably be:
>
> 'yes' means default behaviour, that is add cover letter for
> multiple-patch series, non for a single patch;
>
> 'no' means no cover; and
>
> 'always' means a probably insane "cover even a single patch".
>
> In any case, because this new feature is way too late to be in the
> upcoming 1.6.3 release anyway, I think it is a saner approach to add a
> command line option "--cover=yes" to "cover if multiple", "--cover=always"
> to "cover even a single patch", and "--cover=no" to countermand a
> configured "format.cover" the user may have in the configuration from the
> command line.
You're right. And so is Jeff.
I'll send a followup, that will address comments of the both of you.
See below.
>
> > overwritecoverletter = false
>
> I do not think it is particularly a good idea, and it is a good idea to
> have it in the configuration.
>
> - Why not protect the earlier patch output? People often tweak messages
> (both above and below the three-dash lines) in them.
>
> - Isn't this pretty much per invocation?
>
> I can understand (I may not be enthused about it) a new --clobber={yes,no}
> command line option to allow/forbid clobbering the existing files, and you
> may want to add --clobber=patches to allow clobbering only the patches but
> not cover (which I do not think makes much sense, though).
This was a followup idea, which I thought was a reasonable. In the
meantime, I feel this is not needed. I'd agree that a more generic
--clobber option would make sense but frankly, protecting overwrites at
this stage is not something worth putting much effort into, IMHO.
Jeff King <peff@xxxxxxxx> wrote:
> On Sat, Apr 18, 2009 at 06:16:15PM +0200, Frank Terbeck wrote:
[...]
> Something about "coveronepatch" seems a bit hack-ish to me. Perhaps it
> should be "generate cover letter if there are more than N patches". You
> could even just overload "format.coverletter" as:
>
> true - always generate cover letter
> false - never generate cover letter
> <number> - generate if there are at least <number> patches
Yeah, a lot better.
As Junio said, noone wants cover letters for single patches.
Therefore, I made format.coverletter default to 2. Setting it to one
makes --cover-letter produce for cover letter for single patches, too.
Setting it to 0 disables cover letters, even with --cover-letter. Any
other value sets the minimum size of patch series that will trigger
--cover-letter to create a cover letter.
That can be altered from the command-line via:
--cover-letter=always
--cover-letter=never
--cover-letter=<length>
In addition to that I did add format.coverauto.
Setting it to true (it defaults to false) makes format-patch behave like
the user gave the --cover-letter option - with the following exception:
if --stdout is specified, cover letters are disabled. To force cover
letters in that case too, --cover-letter needs to be specified *after*
--stdout.
Now that I think of it again, two weeks after writing this mail
originally, I guess it would be possible to drop format.coverauto
altogether and tell users to use:
% git config --global alias.fp format-patch --cover-letter
I don't know which solution would be preferred. If it's the user-should-
use-an-alias approach, I'll create a series that gets rid of
format.coverauto changes.
[...]
> > The series is based on master and doesn't seem to break anything
> > within the test suite. It could maybe use own tests, but I must admit
> > that I didn't look too closely at how git's test suite works and where
> > to put in tests for this.
>
> The tests below were not actually run (and you can see they are based on
> what I proposed above, not your existing patches), but they should give
> hopefully give you a headstart.
Yes, it did. Thank you.
Frank Terbeck (4):
The new options and changes:
Add format.coverletter option
Add format.coverauto boolean
Tests, thanks to Jeff:
Add tests for coverauto, coverletter and --cover-letter
And finally documentation.
Documentation for format.coverletter and --cover-letter
Documentation/config.txt | 9 +++++
Documentation/git-format-patch.txt | 17 ++++++++--
builtin-log.c | 32 +++++++++++++++++--
t/t3400-rebase.sh | 1 +
t/t4014-format-patch.sh | 58 ++++++++++++++++++++++++++++++++++-
5 files changed, 108 insertions(+), 9 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html