From: Junio C Hamano <gitster@pobox.com>
To: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
Taylor Blau <me@ttaylorr.com>,
Derrick Stolee <derrickstolee@github.com>
Subject: Re: [PATCH RESEND] format-patch: add --description-file option
Date: Mon, 07 Aug 2023 23:33:49 -0700 [thread overview]
Message-ID: <xmqqttta9h6a.fsf@gitster.g> (raw)
In-Reply-To: 20230807170936.2336760-1-oswald.buddenhagen@gmx.de
Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
> When formatting patches from a detached HEAD, there is no branch
> description to derive the cover letter from. While with format-patch
> one could post-process the generated file (which would be ugly enough),
> scripting that with send-email would be *really* ugly. So add an option
> to feed a description directly.
I think it makes sense to give the same set of features to those who
run format-patch from a detached HEAD as to those who run it on a
branch. But personally I am not interested in a new feature that
encourages use of send-email as a front-end to format-patch, which I
consider is a misfeature, to make it easier for a set of patches
without final proofreading to be sent out.
Having said that, with my maintainer hat on, if we were to add a new
feature to format-patch, it makes sense to allow it passed through
send-email as well, since the (mis)feature already exists.
Please elaborate a bit more on the use case, though.
* "there is no branch description to derive from" makes a reader
wonder what the workflow would become if you could do "git branch
--add-description HEAD" to prepare a description, which would
imply that what is more desirable might be a feature enhancement
of the "branch" command, not "format-patch" or "send-email", to
allow you to describe what you are doing on the HEAD.
* Or does the end-user have a branch with description already
prepared, but for some untold reason the tip of the branch is
checked out on a detached HEAD?
If so, an obviously better alternative design would be to add a
feature that passes a branch name to format-patch and tell it to
pretend that the user is working on the branch. That way, not
just "description", any feature that makes the command use "which
branch are we on?" information to enhance its behaviour we have
right now or we will add to the command will all benefit. For
example, builtin/log.c::cmd_format_match() uses branch_name only
for calling read_branch_desc() via prepare_cover_text(), but it
is perfectly reasonable for us to make the range-diff default
derived based on the reflog of the "current branch" on, and
"pretend we were on this branch" may help you in such a case.
In other words, if a particular solution proposed (or not proposed)
is sensible or not heavily depends on how the end-user ends up
running format-patch (and sending the output out) on a detached
HEAD, and where does the end-user want to take the description
information from. No, the answer to the latter is not "the file
specified with the --description-file option"; that is not a valid
answer. The question is about how the contents of that file is
populated and maintained.
A feature to specify the template used when generating the cover
letter may also work well for such a use case. Among placeholders
to specify where to place auto-generated things like:
- shortlog information
- other ways to list commits in the series (e.g. listing of commit
titles from "git log --oneline -r" may be more appropriate and
readable than "shortlog" output especially when the series was
written by multiple authors),
- diffstat
there would be a placeholder to stuff branch description output (for
the normal case), and in your detached HEAD use case, you'd prepare
such a template without using branch description placeholder, but
instead prepare the description in place in the template before
running format-patch. Which might actually be a better alternative.
But all of that depends on what the expected use case to support is.
Thanks.
next prev parent reply other threads:[~2023-08-08 16:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-07 17:09 [PATCH RESEND] format-patch: add --description-file option Oswald Buddenhagen
2023-08-08 6:33 ` Junio C Hamano [this message]
2023-08-09 17:15 ` [PATCH v2] " Oswald Buddenhagen
2023-08-11 21:38 ` Junio C Hamano
2023-08-11 22:29 ` Oswald Buddenhagen
2023-08-12 8:21 ` Junio C Hamano
2023-08-13 8:22 ` Oswald Buddenhagen
2023-08-21 17:07 ` [PATCH v3] " Oswald Buddenhagen
2023-08-21 17:59 ` Junio C Hamano
2023-08-21 22:05 ` Junio C Hamano
2023-08-23 20:01 ` Taylor Blau
2023-09-23 22:14 ` Kristoffer Haugsbakk
2023-09-25 19:01 ` Junio C Hamano
2023-09-25 19:29 ` Kristoffer Haugsbakk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqttta9h6a.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--cc=oswald.buddenhagen@gmx.de \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).