[PATCH] Avoid warning when From: is encoded

Previous thread: [RFC] custom strategies in builtin-merge by Miklos Vajna on Friday, July 25, 2008 - 4:33 am. (6 messages)

Next thread: Re: [PATCH] Avoid warning when From: is encoded by Junio C Hamano on Friday, July 25, 2008 - 9:33 am. (2 messages)
From: Peter Valdemar Mørch
Date: Friday, July 25, 2008 - 6:06 am

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

--

From: Abhijit Menon-Sen
Date: Friday, July 25, 2008 - 6:16 am

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

From: Sverre Rabbelier
Date: Friday, July 25, 2008 - 9:01 am

Usually the "comment" part is indented by at least one level, but

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

From: Jon Loeliger
Date: Friday, July 25, 2008 - 9:39 am

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

Agreed.

jdl

--

From: Johannes Schindelin
Date: Friday, July 25, 2008 - 7:53 pm

Hi,


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

--

Previous thread: [RFC] custom strategies in builtin-merge by Miklos Vajna on Friday, July 25, 2008 - 4:33 am. (6 messages)

Next thread: Re: [PATCH] Avoid warning when From: is encoded by Junio C Hamano on Friday, July 25,