Re: [PATCH] mailinfo: Remove only one set of square brackets

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 29, 2009 at 11:55:51AM +0200, Andreas Ericsson wrote:

> git-format-patch prepends patches with a [PATCH x/n] prefix, but
> mailinfo used to remove any number of square-bracket pairs and
> the content between them. This prevents one from using a commit
> subject like this:
> 
>   [ and ] must be allowed as input
> 
> Removing the square bracket pair from this rather clumsily
> constructed subject line loses important information, so we must
> take care not to.
> 
> This patch causes the subject stripping to stop after it has
> encountered one pair of square brackets.

I think this is a definite improvement, though I would be much more
convinced that the does the right thing if there were some tests. :)

> One possible downside of this patch is that the patch-handling
> programs will now fail at removing author-added square-brackets
> to be removed, such as
> 
>   [RFC][PATCH x/n]
> 
> However, since format-patch only adds one set of square brackets,
> this behaviour is quite easily undesrstood and defended while the
> previous behaviour is not.

Agreed. And I think Junio raised a good point elsewhere: there are
certain formatting conventions that are part of format-patch output. So
I think we do need to address "this subject munging is totally idiot
proof and will always reproduce the input patch text exactly". But
rather "is this a sane and useful way to do the munging?". And I think
it is a useful convention.

This is a user-visible change that might impact people's workflows (if
only slightly), though, so it should probably get a good mention in the
release notes.

-Peff
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]