Junio C Hamano wrote:
Jeff King <peff@xxxxxxxx> writes:
On Sun, Jun 28, 2009 at 08:38:58PM +0100, Roger Leigh wrote:
In most of the projects I work on, the git commit message has
the affected subsystem or component in square brackets, such as
[foo] change bar to baz
[...]
The [sbuild] prefix has been dropped from the Subject, so an
important bit of context about the patch has been lost.
It's a bit of a bug that you can't round trip from a git-format-patch
to import with git-am and then not be able to produce the exact same
patch set with git-format-patch again (assuming preparing and applying
to the same point, of course).
As an immediate solution, you probably want to use "-k" when generating
the patch (not to add the [PATCH] munging) and "-k" when reading the
patch via "git am" (which will avoid trying to strip any munging).
However:
Would it be possible to change the git-mailinfo logic to use a less
greedy pattern match so it leaves everything after
([PATCH( [0-9/])+])+ in the subject? AFAICT this is cleanup_subject in
builtin-mailinfo.c? Could this rather complex function not just do a
simple regex match which can also take care of stripping ([Rr]e:) ?
Yes, I think in the long run it makes sense to strip just the _first_
set of brackets. I don't think we want to be more specific than that in
the match, because we allow arbitrary cruft inside the brackets (like
"[RFC/PATCH]", etc). But if format-patch always puts exactly one set of
brackets, and am strips exactly one set, then that should retain your
subject in practice, even if it starts with [foo].
I think it may still make sense to insist that PATCH appears somewhere in
the first set of brackets, but I have stop and wonder if it is even
necessary.
Because git removes [sbuild] at the beginning, Roger is unhappy.
[ and a lot more ]
_An_ established (note that I did not say _the_ nor _best current_)
practice supported well by git to note the area being affected in a
project of nontrivial size is to prefix the single line summary with the
name of the area followed by a colon. There is no difference between
"[sbuild] foo" and "sbuild: foo" at the information content point-of-view,
but the latter has an advantage of being one letter shorter and less
distracting in MUA. He does not have a very strong reason to choose
something different only to make his life harder, does he?
True, but it seems wrong to have am remove more of the subject than
format-patch prepends. Imagine a commit subject looking like this:
"Allow [ and ] in the blurble.foostuff table".
Should am strip the subject all the way up to the last ']'? I think
not, and I'd be very vexed if it did.
Users can take advantage of this established practice when running
shortlog with "--grep=^area:" to limit the birds-eye-view to a specific
area. If this turns out to be useful, we could even add an option to "git
log --area=name" that limits this kind of match to the first paragraph of
the commit log message, for example.
Supporting a slightly different convention may seem to be accomodating and
nice, but if there is no real technical difference between the two (and
again, "area:" is one letter shorter ;-), letting people run with
different convention longer, when they can switch easily to another
convention that is already well supported, may actually hurt them in the
long run. "[sbuild]" will not match "--area=sbuild" that will internally
become "--grep-only-first-line=sbuild:" so either he will miss out
benefiting from the new feature, or the implementation of the new feature
unnecessarily needs more code.
It is not about discouraging a wrong workflow or practice, because there
is nothing _wrong_ per-se in [sbuild] prefix. It is just that it makes
things harder in the long run. In this particular case, it is only very
slightly harder, but these things tend to add up from different fronts.
Agreed, but there are valid use-cases orthogonal to subsystem naming to
place [] in the patch subject. I still feel that since format-patch only
adds one set, am (mailinfo) should really only remove one set, too. It's
what makes sense, really.
--
Andreas Ericsson andreas.ericsson@xxxxxx
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
Considering the successes of the wars on alcohol, poverty, drugs and
terror, I think we should give some serious thought to declaring war
on peace.
--
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