Re: [RFC PATCH] builtin-log: Add options to --coverletter

Joe Perches <joe@perches.com> writes:

I think it makes sense to let users affect how the short-log in the cover letter is generated. I do not think overloading the --cover-letter option for doing it is the ideal approach, though.

 - Currently we never generate a cover without being asked, but if we ever
   switch that default in the future, we would want to be able to decline
   it from the command line with --cover-letter=no (or --no-cover-letter).
   The "no" here is different from "please do not wrap but do produce
   cover letter".

 - Currently there is only one style of cover letter, but people may want
   to add a mechanism to let them use different styles that suit their
   project better in the future, and a natural syntax to ask for a
   different style is --cover-letter=style, where "style" may be
   "default", "shortlog", or some other token that the user invents.
   --cover-letter=no will still ask to suppress cover letters.

This is a tangent, but I do not think the current cover-letter that uses shortlog matches everybody's needs. The shortlog format lists commits grouped by the author and does not number them, and it makes it hard to match which message in the series corresponds to which entry in the cover letter, especially when your series have a resend of somebody else's patch in it. I wouldn't be surprised if somebody comes up with a different style that is based on "git log --reverse --oneline A..B" output (perhaps without the shortened object name part) and name it the "oneline" style, e.g.

    From: Jeff King

    *** BLURB HERE ***
    The following patches do ...

    1/2	parseopt: add OPT_NEGBIT (Réne Scharfe)
    2/2 ls-files: make --no-empty-directory negatable

     Documentation/technical/api-parse-options.txt |    4 +++
     ...
     test-parse-options.c                          |    1 +
     6 files changed, 45 insertions(+), 3 deletions(-)

Now, where does "line wrapping parameters" fit in the picture of this possible future with multiple styles? From the implementation convenience viewpoint, you could make the line wrapping knobs specific to the "shortlog" style. It however is conceivable that "oneline" style (yet to be written by somebody) may want to wrap its output, and the parameters it would want to use may well be the same set.

I think it would make sense to introduce a separate parameter:

	--cover-letter-wrap=<pos>,<indent>,<wrap-offset>

at this point in your patch. It does not add oneline or any other different styles, but it would keep the door open for later additions without breaking the UI.

By the way, don't people find the semantics of in1/in2 to shortlog wrapping unnatural? By the above three-tuple parameter, I meant:

	wrap at position [pos], indent the whole thing by this much
	[indent], and offset the second and subsequent lines by this much
	[wrap-offset].

and that reads much better than the -w option of shortlog that says

	wrap at position [pos], indent the whole thing by this much
	[in1], and offset the second and subsequent lines by this much
	[in2 minus in1].