FEATURE REQUEST: git-format-path: Add option to encode patch content

FEATURE REQUEST: git-format-path: Add option to encode patch content

[Jari Aalto]

FEAURE REQUEST
--------------

Add sme option to git-format-path to encode thae patch content 
(MIME/base64)

RATIONALE
-------------

I need to submit patches that fix trailing whitespcaes, and to guard
those in email message it would be nice if the patch could be made
into base64 envelope (or MIME)

-- Jari

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Johannes Schindelin]

Hi,

On Fri, 27 Apr 2007, Jari Aalto wrote:

> FEAURE REQUEST
> --------------
> 
> Add sme option to git-format-path to encode thae patch content 
> (MIME/base64)

Although you ask politely, and have a good explanation why you need it, 
there is a strong sentiment in some Git developers (most notably, Git's 
inventor) against this kind of support.

You might have more luck when you decide to implement it yourself, and 
submit it, though (if it is not too intrusive, I see people adding it, if 
only to add even more rope to those willing to hang themselves).

Having said that, I think that you can actually teach every mail app 
(except maybe web-based ones) to respect formatting, including white 
space.

Ciao,
Dscho

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Jari Aalto]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Fri, 27 Apr 2007, Jari Aalto wrote:
>
>> FEAURE REQUEST
>> --------------
>> 
>> Add sme option to git-format-path to encode thae patch content 
>> (MIME/base64)
>
> Although you ask politely, and have a good explanation why you need it, 
> there is a strong sentiment in some Git developers (most notably, Git's 
> inventor) against this kind of support.
>
> You might have more luck when you decide to implement it yourself, and 
> submit it, though (if it is not too intrusive, I see people adding it, if 
> only to add even more rope to those willing to hang themselves).
>
> Having said that, I think that you can actually teach every mail app 
> (except maybe web-based ones) to respect formatting, including white 
> space.

I hope the development team considers this. There is another uses case
that I can think of too:

- Sending non us-ascii patches (different langauge)

For documentation or code (Java/XML allows UTF-8). 

Jari

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Alex Riesen]

Jari Aalto, Fri, Apr 27, 2007 19:33:35 +0200:
> > Having said that, I think that you can actually teach every mail app 
> > (except maybe web-based ones) to respect formatting, including white 
> > space.
> 
> I hope the development team considers this. There is another uses case
> that I can think of too:
> 
> - Sending non us-ascii patches (different langauge)

not a good use case: you can send text/plain mails with UTF-8 content
just as easily. And the patches will apply.

P.S. You ignored the suggestion to implement base64. Why?

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Junio C Hamano]

Jari Aalto <jari.aalto@cante.net> writes:

> FEAURE REQUEST
> --------------
>
> Add sme option to git-format-path to encode thae patch content 
> (MIME/base64)

One possibility is to have a separate mail sending or formatting
program that works on format-patch output.  In any case, I feel
that _generating_ base64 natively does not belong to core git.
We _tolerate_ MIME in mailinfo, but that does not mean we
encourage MIME.  Keeping patches reviewable in recipient's MUA
is important.

Now, after having given the "official" party-line answer, I
personally think it would make sense to always do base64 when
attaching the patch (i.e. "--attach", not "--inline").

In the community git originates from, even --inline attachments
are frowned upon, let alone --attach multiparts (yes, I am
talking about the kernel list and this list).  Nobody who sends
patches over e-mail in communities that have a tradition against
unreadable patches would be using --inline nor --attach anyway,
so making --attach to do base64 would not hurt there.

On the other hand, in circles whose recipients are prepared to
(or even "prefer to") accept messages formatted with --attach,
there should be no harm done if the patch text is base64
encoded either.

Am I going to do it?  Not likely.  But if a patch is cleanly
done, I do not think there is much reason to reject one.

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Robin H. Johnson]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Fri, Apr 27, 2007 at 12:49:02PM -0700, Junio C Hamano wrote:
> On the other hand, in circles whose recipients are prepared to
> (or even "prefer to") accept messages formatted with --attach,
> there should be no harm done if the patch text is base64
> encoded either.
+1 for having this feature on my end.

I do some work on other projects where patches that aren't multipart
aren't even accepted (the author has an applying system that he bounces
the patches into, and refuses to change it :-( ) [again, projects that
aren't git upstream, but I track with git because that makes development
much easier].

-- 
Robin Hugh Johnson
Gentoo Linux Developer & Council Member
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

[-- Attachment #2: Type: application/pgp-signature, Size: 321 bytes --]

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1613 bytes --]

On Fri, 27 Apr 2007 12:49:02 -0700, Junio C Hamano wrote:
> Keeping patches reviewable in recipient's MUA is important.

Absolutely.

> In the community git originates from, even --inline attachments
> are frowned upon, let alone --attach multiparts (yes, I am
> talking about the kernel list and this list).  Nobody who sends
> patches over e-mail in communities that have a tradition against
> unreadable patches would be using --inline nor --attach anyway,
> so making --attach to do base64 would not hurt there.

I know I'm not within the kernel or git party line by saying this, but
I don't have a problem with (non base64) mime-attached patches. My MUA
displays them fine and quotes them when I reply so I can easily review
them.

I have even wished that git-am would accept a message with multiple
attachments and apply each in series. But, no, I'm not about to write
that myself either.

Instead we've gotten by with using the output of "git format-patch
--stdout from..to > feature.patchset" and attaching that. But even
then, I'd like something like git-am that knew how to process an email
message like that, (that is, not trying to use the subject and from
lines for commit information). As is, I just have to arrange for the
attachment to get piped to git-am instead of the entire message.

If nothing else, there's a datapoint from a separate community that
also has a tradition against unreadable patches.

My MUA doesn't have support (that I know of) for piping a sequence of
messages to a command. Do your MUAs? If not, how do people generally
handle long sequences of [n/N] patch emails?

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Junio C Hamano]

Carl Worth <cworth@cworth.org> writes:

> My MUA doesn't have support (that I know of) for piping a sequence of
> messages to a command. Do your MUAs? If not, how do people generally
> handle long sequences of [n/N] patch emails?

Are you asking _me_ specifically?  Assuming so,

Yes, my MUA does allow "next N messages" or "selected messages"
to be given to an external program.

But I typically save them in a separate mailbox for reviewing
and in-editor fixups before applying, so "next N messages" are
useful to me for saving but not applying.

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Linus Torvalds]

On Fri, 27 Apr 2007, Carl Worth wrote:
> 
> My MUA doesn't have support (that I know of) for piping a sequence of
> messages to a command. Do your MUAs? If not, how do people generally
> handle long sequences of [n/N] patch emails?

Well, in pine you *can* actually select a long sequence of messages, and 
then "a|" to "apply a pipe" to them, and ask for them to be sent as-is, 
and thus push them directly to git-am if you wanted to.

But quite frankly, I've never done it. It's _much_ easier to just select 
the messages, do "as" to "apply save", and just save them to a new mbox, 
and them apply that mbox in another xterm.

One reason I do that is that while I've trained my cadre of killer kernel 
development monkeys to send me emails in the proper format, I quite often 
end up editing away things like "Hi, please apply" and "thanks" etc. And I 
actually add my sign-offs manually.

(Of course, my "manual" sign-off is often a "search-and-replace" of the 
previous sign-off, so that I not only don't have to type that much, but 
also so that my workflow automatically guarantees that I only sign off on 
stuff that was signed off by the sender, and I notice if my 
search-and-replace says that it replaced 213 entries, even though I 
expected to have 214 emails there..)

I'm sure there are mail clients that don't support even that kind of basic 
automation ("Please save each email individually! Because mousing around 
is good for you! It's the only exercise you get!"), but I can't imagine 
that anybody uses them. So the "save to a file, go through it there, fix 
up, and apply" sequence is a pretty good one. And it scales really well.

(By "scales really well", I mean that I usually end up doing series of 
just a few emails. But occasionally I have batches either from a single 
user, or multiple different people who I batch up in one go, and I do tens 
and even hundreds of patches in one "go" - I do *not* want to switch 
back-and-forth between the email client and another xterm, but at the same 
time, email clients really aren't generally set up to doing things like 
search-and-replace etc on hundreds of messages at a time).

I'm sure if you use GNU emacs to read email, you could do it all within 
that emacs client, but I couldn't help you with that anyway.

		Linus

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]

On Fri, 27 Apr 2007 14:40:33 -0700 (PDT), Linus Torvalds wrote:
> But quite frankly, I've never done it. It's _much_ easier to just select
> the messages, do "as" to "apply save", and just save them to a new mbox,
> and them apply that mbox in another xterm.

OK. That's basically exactly the same mode I work with, (but I prefer
the user to just send me that mbox file as a single attachment rather
than broken out into separate messages). If nothing else, it
eliminates the "select the messages" step.

And, sure I can see that where you need to aggregate dozens (or
hundreds or whatever) of separate patches to apply in one go, it's
really nice that you can do that. Personally, I'm not in that
situation myself. Though if I were, I'd still be happy to have each
patch-set submission be pre-aggregated for me. Maybe you're getting
that already with a single "select thread" operation or whatever.

Anyway, thanks for satisfying some idle curiosity on my part.

-Carl

PS. A separate reason I like attachments is that I like the subject
and message body to be the actual discussion of the patch, (and the
commit message and patch content to come later). I find that, (as
opposed to commentary being inserted between commit message and patch
content), more natural when a discussion on the mailing list turns
into a patch attempt. But I fully recognize this is a cultural
separation between my project and that of the kernel and git
projects---certainly nothing needing any debate.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Linus Torvalds]

On Fri, 27 Apr 2007, Carl Worth wrote:
> 
> PS. A separate reason I like attachments is that I like the subject
> and message body to be the actual discussion of the patch, (and the
> commit message and patch content to come later).

And the reason I _hate_ attachements is that you cannot say "no, that one 
is wrong", and just skip/edit it.

Because once you start doing attachments, you're not going have just one 
patch per email. You're going to have people send you ten attachments in 
one email for patches 1-10.

And when patch 5 is crap, you're basically screwed. There is almost no 
support for replying to that _individually_ and saying "That one sucks". 
You press "reply", and you get all ten.

So the fundamental rule *must* be: one patch, one email. That's the 
starting point. Anything else is simply broken. 

And once you accept that fundamental rule (and quite frankly, I don't care 
if others accept it or not - I'm personally 100% convinved that anythign 
else is a major mistake), suddenly attachments have no upsides at all, 
only downsides. They mean that when you send the patch top a mailing list, 
everybody sees it differently, and a *lot* of people will be unable to 
reply to it with proper quoting.

		Linus

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Linus Torvalds]

On Fri, 27 Apr 2007, Linus Torvalds wrote:
>
> And when patch 5 is crap, you're basically screwed. There is almost no 
> support for replying to that _individually_ and saying "That one sucks". 
> You press "reply", and you get all ten.

Btw, this is true even when the attachment itself is just an mbox.

Yes, you can (and probably do) just save the mbox to a file, and then 
start _another_ mail reader on that file. That's actually what I end up 
doing a lot for my "doit" file (which is just where I save all the 
messages that I'll apply) when I go through them more carefully.

But it doesn't change the fact that if you just see something wrong, you 
can't just reply to it and say "that's crap". You have to basically exit 
your primary mail-reader, and start a secondary one on the file you saved.

Also, I assume that this means that nobody else is ever Cc'd on the 
patches you get? Like a public mailing list?

		Linus

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1207 bytes --]

On Fri, 27 Apr 2007 16:41:12 -0700 (PDT), Linus Torvalds wrote:
> And the reason I _hate_ attachements is that you cannot say "no, that one
> is wrong", and just skip/edit it.

Well, skipping it while applying the rest is bad form anyway, isn't
it? I think "bounce the whole series on any problem" reduces load on
the maintainer and helps the submitter learn by fixing up the patch
series personally rather than trusting the maintainer to do it.

> And when patch 5 is crap, you're basically screwed. There is almost no
> support for replying to that _individually_ and saying "That one sucks".
> You press "reply", and you get all ten.

As soon as there's anything broken in the series, the patch boundaries
aren't that interesting to me anymore. At that point, I just reply to
the whole thing and strip the reply down to the stuff worth commenting
on.

So, my style is accept or bounce, and during the bounce, I'll just
review the whole thing while quoted in my reply buffer.

That seems to work for me anyway, without a one-patch-per email
approach, (in fact, one-patch-per-email would be a lot more awkward
for replying to the whole series and quoting similar or related issues
as I like to do).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Linus Torvalds]

On Fri, 27 Apr 2007, Carl Worth wrote:
> > And the reason I _hate_ attachements is that you cannot say "no, that one
> > is wrong", and just skip/edit it.
> 
> Well, skipping it while applying the rest is bad form anyway, isn't
> it?

Why would it be?

Maybe for small projects with strictly linear development, but I get a 
_lot_ of patch-series where the patches are independent of each other. 
Andrew's patch-series are one example, but so is a lot of the "trivial 
tree" patch-series too.

			Linus

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Fri, 27 Apr 2007, Carl Worth wrote:
>> > And the reason I _hate_ attachements is that you cannot say "no, that one
>> > is wrong", and just skip/edit it.
>> 
>> Well, skipping it while applying the rest is bad form anyway, isn't
>> it?
>
> Why would it be?
>
> Maybe for small projects with strictly linear development, but I get a 
> _lot_ of patch-series where the patches are independent of each other. 
> Andrew's patch-series are one example, but so is a lot of the "trivial 
> tree" patch-series too.

Seconded, even for a small project like git.git, I do that all
the time.

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Junio C Hamano]

Carl Worth <cworth@cworth.org> writes:

>> In the community git originates from, even --inline attachments
>> are frowned upon, let alone --attach multiparts (yes, I am
>> talking about the kernel list and this list).  Nobody who sends
>> patches over e-mail in communities that have a tradition against
>> unreadable patches would be using --inline nor --attach anyway,
>> so making --attach to do base64 would not hurt there.
>
> I know I'm not within the kernel or git party line by saying this, but
> I don't have a problem with (non base64) mime-attached patches. My MUA
> displays them fine and quotes them when I reply so I can easily review
> them.

You strayed into tangent here.  You said you do not have a
problem with non base64 attachments.  I would like to know if
you have or do not have problem with base64 ones, as that is
what indicates your opinion on making --attach unconditionally
do base64 which was the suggestion made in the thread.

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Johannes Schindelin]

Hi,

On Fri, 27 Apr 2007, Jari Aalto wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Fri, 27 Apr 2007, Jari Aalto wrote:
> >
> >> FEAURE REQUEST
> >> --------------
> >> 
> >> Add sme option to git-format-path to encode thae patch content 
> >> (MIME/base64)
> >
> > [...]
> >
> > Having said that, I think that you can actually teach every mail app 
> > (except maybe web-based ones) to respect formatting, including white 
> > space.
> 
> I hope the development team considers this. There is another uses case
> that I can think of too:
> 
> - Sending non us-ascii patches (different langauge)
> 
> For documentation or code (Java/XML allows UTF-8). 

That is no counterargument. I.e. it does not hold through basic scrutiny.

Ciao,
Dscho

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 1880 bytes --]

On Fri, 27 Apr 2007 17:56:46 -0700, Junio C Hamano wrote:
> You strayed into tangent here.  You said you do not have a
> problem with non base64 attachments.

Yes, sorry for that.

>                                      I would like to know if
> you have or do not have problem with base64 ones, as that is
> what indicates your opinion on making --attach unconditionally
> do base64 which was the suggestion made in the thread.

I definitely have a problem with base64-encoded patches.

My MUA does much of exactly what I want with text attachments, (and
stuff I thought most any MUA would do). Specifically, it always
displays text attachments immediately following the message body
itself. And it also quotes text attachments when I reply.

So, for me, a text attachment is almost identical in usability to an
inline patch. And in some ways, it's even more usable. Since my MUA
knows about the boundaries of each attachment, then if there are multiple
patches, I can operate on any one individually (saving or organizing,
or whatever), or even "reply to" an individual attachment by quoting
only that one.

And my MUA does all that even if the text attachment comes with an
"attachment" disposition instead of an "inline" disposition. And I
can't imagine why any MUA would not immediately display a text
attachment, (unless as Linus suggests, it's to encourage user's to get
better exercise by clicking more).

*And* I think my MUA even does all that even if the attachment comes
base64-encoded. But in spite of that, I still am not happy to ever
receive a base64-encoded patch. Because I use things other than my MUA
to look at my mail, (for example, I use grep and sometimes other
indexing tools over my mail, and the base64 encoding ruins them).

So, that's where my preference comes for not wanting base64 encoding,
(even when patches come attached but not inline).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On Fri, 27 Apr 2007 17:20:48 -0700 (PDT), Linus Torvalds wrote:
> Why would it be?
>
> Maybe for small projects with strictly linear development, but I get a
> _lot_ of patch-series where the patches are independent of each other.
> Andrew's patch-series are one example, but so is a lot of the "trivial
> tree" patch-series too.

Ah. So here we were just using different definitions of "patch
series". I was using it the sense of a series of patches that were
strongly dependent. I definitely agree with you that independent
patches belong in separate email messages. No doubt about that.

-Carl


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: FEATURE REQUEST: git-format-path: Add option to encode patch content

[Junio C Hamano]

Carl Worth <cworth@cworth.org> writes:

> On Fri, 27 Apr 2007 17:56:46 -0700, Junio C Hamano wrote:
>> ...
>>                                      I would like to know if
>> you have or do not have problem with base64 ones, as that is
>> what indicates your opinion on making --attach unconditionally
>> do base64 which was the suggestion made in the thread.
>
> I definitely have a problem with base64-encoded patches.

Thanks, and I agree with everything you said why base64 is
inconvenient.  So _if_ somebody does an enhancement to do
base64, it needs to be a separate option.

Honestly speaking, though, I think that kind of "attachment"
should be left to MUA, not format-patch.  If somebody knows his
MUA has a tendency to corrupt in-line text, tell the MUA to
attach the format-patch output stored in the filesystem as an
application/octet-stream attachment.  MUA may or may not do
base64 or QP but at that point the only thing the user cares
about is a byte-for-byte faithful reproduction at the
recipient's end, and he does not worry a whit about
readability), so either base64 or QP would suffice.  And the
format-patch does not have to care.