[PATCH] Avoid warning when From: is encoded

[PATCH] Avoid warning when From: is encoded

[Peter Valdemar Mørch]

From: Peter Valdemar Mørch <peter@morch.com>

In commit 0706bd19ef9b41e7519df2c73796ef93484272fd $1 is used from a regexp
without using () to set up $1. Later, when that value was used, it caused a
warning about a variable being undefined.

Signed-off-by: Peter Valdemar Mørch <peter@morch.com>
---
The commit introduces $body_encoding and: $body_encoding = $1; which is undef.

That commit then later uses $body_encoding only here:
+ if ($has_content_type) {
+         if ($body_encoding eq $author_encoding) {
+                 # ok, we already have the right encoding
+         }
+         else {
+                 # uh oh, we should re-encode
+         }
+ }
(I removed some whitespace for readability)

.. and it was the eq that gave the warning, because $body_encoding was
undefined. Perhaps a better fix is to remove $body_encoding and regexp
altogether since it isn't really used. Let me know if you think so.

This is where my non-commit message goes, yeah? I'm hand editing the output of
'git format-patch'...

Junio C. Hamano commented on a previous post that I shouldn't send patches as
attachments so now I'm trying git-send-email. Are there any form problems with
this patch?

 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 2e4a44a..d2fd899 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -882,7 +882,7 @@ foreach my $t (@files) {
 				}
 				elsif (/^Content-type:/i) {
 					$has_content_type = 1;
-					if (/charset="?[^ "]+/) {
+					if (/charset="?([^ "]+)/) {
 						$body_encoding = $1;
 					}
 					push @xh, $_;
-- 
1.6.0.rc0.46.g07955.dirty

Re: [PATCH] Avoid warning when From: is encoded

[Abhijit Menon-Sen]

At 2008-07-25 15:06:48 +0200, 4ux6as402@sneakemail.com wrote:
>
> This is where my non-commit message goes, yeah?

Yes.

> Are there any form problems with this patch?

Looks fine to me (and also to git am).

The patch itself also looks good to me (but I'm not sure if that means I
should add an Acked-by: line to this message).

-- ams

Re: [PATCH] Avoid warning when From: is encoded

[Sverre Rabbelier]

On Fri, Jul 25, 2008 at 15:16, Abhijit Menon-Sen <ams@toroid.org> wrote:
> At 2008-07-25 15:06:48 +0200, 4ux6as402@sneakemail.com wrote:
>> Are there any form problems with this patch?

Usually the "comment" part is indented by at least one level, but
afaik that's just etiquette and is not mandatory.

> The patch itself also looks good to me (but I'm not sure if that means I
> should add an Acked-by: line to this message).

Acked-by is reserved for people who are "owners" of the area the patch
touches. So for example, a patch to git-gui could be Acked-by Shawn O.
Pierce, or one related to pack format by Nico (I think?). So you
should Ack it if you have done (a lot of) work in the same area as the
patch before and if the patch looks good.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] Avoid warning when From: is encoded

[Jon Loeliger]

Sverre Rabbelier wrote:

> Acked-by is reserved for people who are "owners" of the area the patch
> touches.

I love pronouncements like this.  While that may be exactly true
for the Git project, it is not, in general, always true.  Within
parts of the Kernel development process, anyone who wants to may
ACK a patch if they have done some level of work to confirm that
it "is good", for some measure of "good", even if that is just
applying the patch and testing it.  It is re-assurance that other
people consider the patch acceptable.

Of course, if there are, say, multiple functional areas with
different maintainers, and the patch should go in via one repository
but crosses into a second or third functional area, getting the
ACK from the other maintainers may be considered essential for
its ultimate acceptance.  In that regard, yes, the maintainer's
ACK carries more weight.

> So for example, a patch to git-gui could be Acked-by Shawn O.
> Pierce, or one related to pack format by Nico (I think?). So you
> should Ack it if you have done (a lot of) work in the same area as the
> patch before and if the patch looks good.

Agreed.

jdl

Re: [PATCH] Avoid warning when From: is encoded

[Johannes Schindelin]

Hi,

On Fri, 25 Jul 2008, Jon Loeliger wrote:

> Sverre Rabbelier wrote:
> 
> > Acked-by is reserved for people who are "owners" of the area the patch
> > touches.
> 
> I love pronouncements like this.  While that may be exactly true
> for the Git project, it is not, in general, always true.

It may not be true in general, but from what I heard of the Kernel 
community, even there it is considered rude if you just step in and say 
ACK, when you clearly have no idea what you are talking about (which is 
normally determined by your being involved in that area).

So you can love (or not) pronouncements like that, but the fact still 
stands true: how can your ACK be of any value (or for that matter, how can 
your ACK be taken seriously) when you haven't proven -- in code! -- that 
you understand the code?

Hthab,
Dscho

Re: [PATCH] Avoid warning when From: is encoded

[Junio C Hamano]

Peter Valdemar Mørch  <4ux6as402@sneakemail.com> writes:

> From: Peter Valdemar Mørch <peter@morch.com>
>
> In commit 0706bd19ef9b41e7519df2c73796ef93484272fd $1 is used from a regexp
> without using () to set up $1. Later, when that value was used, it caused a
> warning about a variable being undefined.
>
> Signed-off-by: Peter Valdemar Mørch <peter@morch.com>

Thanks.  The patch is obviously correct; I think you are fixing 8291db6
(git-send-email: add charset header if we add encoded 'From', 2007-11-16),
not 0706bd1 (send-email: specify content-type of --compose body,
2008-03-28).

Will apply to maint and merge upwards.

Re: [PATCH] Avoid warning when From: is encoded

[Jeff King]

On Fri, Jul 25, 2008 at 09:33:56AM -0700, Junio C Hamano wrote:

> > In commit 0706bd19ef9b41e7519df2c73796ef93484272fd $1 is used from a regexp
> > without using () to set up $1. Later, when that value was used, it caused a
> > warning about a variable being undefined.
> >
> > Signed-off-by: Peter Valdemar Mørch <peter@morch.com>
> 
> Thanks.  The patch is obviously correct; I think you are fixing 8291db6
> (git-send-email: add charset header if we add encoded 'From', 2007-11-16),
> not 0706bd1 (send-email: specify content-type of --compose body,
> 2008-03-28).
> 
> Will apply to maint and merge upwards.

Discussions about who can ACK this code aside, the original bogosity was
totally mine, so

  Acked-by: Jeff King <peff@peff.net>

-Peff