Re: [PATCH v3] Add core.mode configuration

[Krzysztof Mazur]

On Tue, Oct 15, 2013 at 11:03:26PM -0500, Felipe Contreras wrote:
  not some next behavior that may change in future.
 
 But I'm suggesting to add a core.addremove option as well, like you suggested,
 am I not?

Yes, I think we both agreed on adding core.addremove. I'm just not
convinced if we should also add core.mode.

 
 So you would be happy if we had core.addremove = true *and* core.mode = next,
 right? You would use one, different people with different needs would use the
 other.

Yes, if there are people that will use core.mode it will be worth
adding. I'm just not one of them.

 
That's safer than next (at least for interactive use) and maybe more 
users
would use that, but I don't think that's worth adding.
   
   Maybe, but I don't think many users would use either mode, and that's 
   good.
   
For me, old behavior by default and warnings with information how to
enable new incompatible features, is sufficient. So I don't need
core.mode option, but as long it will be useful for other users I have
nothing against it.
   
   OK, but that seems to mean you don't need core.mode = next-warn either. 
   I'm not
   against adding such a mode, but I would like to hear about _somebody_ that
   would like to actually use it. I don't like to program for ghosts.
  
  
  As I said earlier, I don't think that next-warn it's worth adding, but
  such option might increase the number of people interested in the
  core.mode.
 
 Well that's a hypothesis, and I would be interested in finding out if that's
 true, but until I see somebody that says I want core.mode = next-war, I'm
 going to assume they are hypothetical.
 

Yes, that's just a hypothesis.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Add core.mode configuration

[Krzysztof Mazur]

On Tue, Oct 15, 2013 at 10:55:07PM -0500, Felipe Contreras wrote:
 John Szakmeister wrote:
  
  I like the idea that we could kick git into a mode that applies the
  behaviors we're talking about having in 2.0, but I'm concerned about
  one aspect of it.  Not having these behaviors until 2.0 hits means
  we're free to renege on our decisions in favor of something better, or
  to pull out a bad idea.  But once we insert this knob, I don't know
  that we have the same ability.  Once people realize it's there and
  start using it, it gets harder to back out.  I guess we could maintain
  the stance that the features are not concrete yet, or something like
  that, but I think people would still get upset if something changes
  out from under them.
 
 We cannot change the behavior of push.default = simple already, so at least
 that option is not in question.

If we add core.addremove=true the same applies to it - we cannot remove
it later, the only we can do is to disable it by default in future
versions after testing (core.addremove=true or core.mode=next).

  So, at the end of the day, I'm just not sure it's worthwhile to have.
 
 This is exactly what happened on 1.6; nobody really tested the 'git foo'
 behavior, so we just switched from one version to the next. If you are not
 familiar with the outcome; it wasn't good.

BTW, I'm still using pre-1.6 git-foo, I have /usr/libexec/git-core
in my PATH. So I would like to always have an option to disable some
new incompatible improvements.

 
 So I say we shouldn't just provide warnings, but also have an option to allow
 users (probably a minority) to start testing this.
 

and an option to keep the old behavior, like we did with push.default.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Add core.mode configuration

[Krzysztof Mazur]

On Mon, Oct 14, 2013 at 04:35:50PM -0500, Felipe Contreras wrote:
 Krzysztof Mazur wrote:
  On Sat, Oct 12, 2013 at 02:04:45AM -0500, Felipe Contreras wrote:
   So that we can specify general modes of operation, specifically, add the
   'next' mode, which makes Git pre v2.0 behave as Git v2.0.
   
   Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
   ---
  
  I don't think that single option it's a good idea. From the user's
  point of view I think that the way push.default was introduced and
  will be changed is much better. So maybe it's better to just add
  core.addremove option instead?
 
 Maybe, but what happens when we start doing changes for v3.0? As a user, I
 don't and to figure out which are the new configurations that will turn v3.0
 behavior on, I just want to be testing that mode, even if I'm not following 
 Git
 development closely. If I find something annoying with core.mode = next, I
 report the problem to the mailing list, which is good, we want to know 
 problems
 with the backward-incompatible changes that will be introduced before it's too
 late, don't we?

But with core.mode = next after upgrade you may experience incompatible
change without any warning. I think it's better to keep the old behavior
by default and warn the user if with new behavior the result might be
different. So the user:

a) knows about the change

b) may set appropriate option to enable the new default or keep
   the old behavior and disable the warning

c) may report that he does not like that change

 
 I'd be fine with having *both* a fine-tuned option to trigger each specific
 behavior, and another one that turns all those fine-tuned options on that are
 meant for v2.0.
 
 Unfortunately, I don't see much interest from Git developers in either.
 

I think that most users have already set the push.default, so git add
is the only problem. If Junio really wants to change git add he should
be interested in allowing user to use it now.

I don't see the change in git add as an improvement, because
removing files with git add IMHO is more confusing than ignoring
such files. Maybe introducing new command - git update for instance -
which is equivalent to new git add and teaching new users to use it
instead of git add is better.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Add core.mode configuration

[Krzysztof Mazur]

On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote:
 Krzysztof Mazur wrote:
  
  But with core.mode = next after upgrade you may experience incompatible
  change without any warning.
 
 Yes, and that is actually what the user wants. I mean, why would the user set
 core.mode=next, if the user doesn't want to experencie incompatible changes? A
 user that sets this mode is expecting incompatible changes, and will be 
 willing
 to test them, and report back if there's any problem with them.

With your patch, because it's the only way to have 'git add' v2.0.
But if another git v2.0 incompatible change will be added it will not
be warned, because with core.mode=next he decided to enable also
future changes and that's why I would never set that.

 
  I think it's better to keep the old behavior by default and warn the user if
  with new behavior the result might be different. So the user:
  
  a) knows about the change
  
  b) may set appropriate option to enable the new default or keep
 the old behavior and disable the warning
  
  c) may report that he does not like that change
 
 But that's what we are doing already. Look at the test I wrote, it's testing
 the warnings for the current version of Git.

With pull.default we did that, but with git add v2.0 now we only warn
the user. With your patch he can enable new git add (and disable warning),
but he also enables future incompatible changes and disables
warnings for such changes. He also cannot keep the old behaviour and
disable the warning.

 
  I don't see the change in git add as an improvement, because
  removing files with git add IMHO is more confusing than ignoring
  such files. Maybe introducing new command - git update for instance -
  which is equivalent to new git add and teaching new users to use it
  instead of git add is better.
 
 I agree. At first I simply ignored the changes because I didn't have the
 patience to figure out what exactly did they mean. Now I was forced to
 understand them to write this patch, and I'm also forcing myself to use this
 behavior.
 
 'git add' removing files is counter-intutive, 'git stage' (currently an alias
 to 'git add') might make more sense.

Yeah, 'git stage' as an alias to 'git add -A' is much more intuitive.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Add core.mode configuration

[Krzysztof Mazur]

On Tue, Oct 15, 2013 at 08:29:56AM -0500, Felipe Contreras wrote:
 Krzysztof Mazur wrote:
  On Tue, Oct 15, 2013 at 07:32:39AM -0500, Felipe Contreras wrote:
   Krzysztof Mazur wrote:

But with core.mode = next after upgrade you may experience incompatible
change without any warning.
   
   Yes, and that is actually what the user wants. I mean, why would the user 
   set
   core.mode=next, if the user doesn't want to experencie incompatible 
   changes? A
   user that sets this mode is expecting incompatible changes, and will be 
   willing
   to test them, and report back if there's any problem with them.
  
  With your patch, because it's the only way to have 'git add' v2.0.
 
 Yeah, but that's not what I'm suggesting. I suggested to have *both* a
 fined-tunned way to have this behavior, say core.addremove = true, and a way 
 to
 enable *all* v2.0 behaviors (core.mode = next).

I'm just not sure if a lot of users would use core.mode=next, because
of possible different behavior without any warning. Maybe we should also
add core.mode=next-warn that changes defaults like next but keeps warnings
enabled until the user accepts that change by setting appropriate
config option? That's safer than next (at least for interactive use) and
maybe more users would use that, but I don't think that's worth adding.

For me, old behavior by default and warnings with information how to
enable new incompatible features, is sufficient. So I don't need
core.mode option, but as long it will be useful for other users I have
nothing against it.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] Add core.mode configuration

[Krzysztof Mazur]

On Sat, Oct 12, 2013 at 02:04:45AM -0500, Felipe Contreras wrote:
 So that we can specify general modes of operation, specifically, add the
 'next' mode, which makes Git pre v2.0 behave as Git v2.0.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---

I don't think that single option it's a good idea. From the user's
point of view I think that the way push.default was introduced and
will be changed is much better. So maybe it's better to just add
core.addremove option instead?

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] Initialize variables with values

[Krzysztof Mazur]

On Thu, May 09, 2013 at 03:13:39AM +0200, Sven Strickroth wrote:
 With MSVC initializing a variable with int a=a causes a warning about
 using an uninitialized value.
 
 Signed-off-by: Sven Strickroth em...@cs-ware.de
 ---
  builtin/rev-list.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/builtin/rev-list.c b/builtin/rev-list.c
 index 67701be..13afacd 100644
 --- a/builtin/rev-list.c
 +++ b/builtin/rev-list.c
 @@ -338,7 +338,7 @@ int cmd_rev_list(int argc, const char **argv, const char 
 *prefix)
   mark_edges_uninteresting(revs.commits, revs, show_edge);
  
   if (bisect_list) {
 - int reaches = reaches, all = all;
 + int reaches = 0, all = 0;
  
   revs.commits = find_bisection(revs.commits, reaches, all,
 bisect_find_all);

But this generates worse code, at least using gcc 4.7.2:

--- old 2013-05-09 14:33:22.0 +0200
+++ new 2013-05-09 14:33:02.0 +0200
@@ -1,2 +1,2 @@
text   data bss dec hex filename
-   4283  0   0428310bb builtin/rev-list.o
+   4299  0   0429910cb builtin/rev-list.o

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] send-email: Honor multi-part email messages

[Krzysztof Mazur]

On Fri, Jan 25, 2013 at 07:28:54PM +0400, Alexey Shumkin wrote:
 git format-patch --attach/--inline generates multi-part messages.
 Every part of such messages can contain non-ASCII characters with its own
 Content-Type and Content-Transfer-Encoding headers.
 But git-send-mail script interprets a patch-file as one-part message
 and does not recognize multi-part messages.
 So already quoted printable email subject may be encoded as quoted printable
 again. Due to this bug email subject looks corrupted in email clients.

I don't think that the problem with the Subject is multi-part message
specific. The real problem with the Subject is probably that
is_rfc2047_quoted() does not detect that the Subject is already quoted.

Of course we still need that explicit multi-part message support to
avoid Which 8bit encoding should I declare [UTF-8]?  message.

 
 diff --git a/git-send-email.perl b/git-send-email.perl
 index 94c7f76..d49befe 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -1499,12 +1499,17 @@ sub file_has_nonascii {
  
  sub body_or_subject_has_nonascii {
   my $fn = shift;
 + my $multipart = 0;
   open(my $fh, '', $fn)
   or die unable to open $fn: $!\n;
   while (my $line = $fh) {
   last if $line =~ /^$/;
 + if ($line =~ /^Content-Type:\s*multipart\/mixed.*$/) {
 + $multipart = 1;
 + }
   return 1 if $line =~ /^Subject.*[^[:ascii:]]/;
   }
 + return 0 if $multipart;
   while (my $line = $fh) {
   return 1 if $line =~ /[^[:ascii:]]/;
   }

After this change the function name is no longer appropriate.
Maybe we should join body_or_subject_has_nonascii()
and file_declares_8bit_cte() because in case of multi-part messages
next unless (body_or_subject_has_nonascii($f)
  !file_declares_8bit_cte($f));
is not valid anymore. We could also check for broken_encoding
in single pass.

Thanks,

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] ignore SIG{INT,QUIT} when launching editor

[Krzysztof Mazur]

On Fri, Nov 30, 2012 at 05:39:43PM -0500, Jeff King wrote:
 This is a re-roll of the pf/editor-ignore-sigint series.
 
 People mentioned some buggy editors which go into an infinite EIO loop
 when their parent dies due to SIGQUIT. That should be a non-issue now,
 as we will be ignoring SIGQUIT. And even if you could replicate it
 (e.g., with another signal) those programs should be (and reportedly
 have been) fixed. It is not git's job to babysit its child processes.
 

Also some good editors printed error message after they got EIO,
confusing the user.

Looks good to me. I've tested this with ed (always ignores SIGINT
and SIGQUIT), vim (always ignores SIGINT, but dies after three
SIGQUIT) and sleep (dies after SIGINT and SIGQUIT) and git works now
as expected. Doing what editor does is probably the best thing to do. 

Tested-by: Krzysztof Mazur krzys...@podlesie.net


Thanks,

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] git-send-email: allow edit invalid email address

[Krzysztof Mazur]

On Mon, Nov 26, 2012 at 03:50:30PM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  On Mon, Nov 26, 2012 at 02:58:58PM -0800, Junio C Hamano wrote:
  Krzysztof Mazur krzys...@podlesie.net writes:
  
   Not having this new code inside elsif (/^e/) { } feels somewhat
   sloppy, even though it is not *too* bad.  Also do we know this
  
   ok, I will fix that.
  
   function will never be used for addresses other than recipients' (I
   gave a cursory look to see what is done to the $sender and it does
   not seem to go through this function, tho)?
  
   Yes, this function is called only from validate_address_just()
   to filter @initial_to, @initial_cc, @bcc_list as early as possible,
   and filter @to and @cc added in each email.
  
  Thanks; when merged to 'pu', this series seems to break t9001.  I'll
  push the result out with breakages but could you take a look?
  
 
  Sorry, I tested final version only on an ancient perl 5.8.8 and it really
  worked there. The third patch is broken:
 
  diff --git a/git-send-email.perl b/git-send-email.perl
  index 9996735..f3bbc16 100755
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -1472,7 +1472,7 @@ sub unique_email_list {
  my @emails;
   
  foreach my $entry (@_) {
  -   my $clean = extract_valid_address_or_die($entry))
  +   my $clean = extract_valid_address_or_die($entry);
 
 Ah, ok, I wasn't looking closely enough.  Thanks for a quick
 turnaround.  Will requeue and push out.

I rechecked that and I've just sent some older broken version. The
patch that I've sent had date Date: Thu, 22 Nov 2012 19:00:25 +0100,
but on my tree I have commit from Date: Thu Nov 22 19:01:55 2012 +0100,
which is exactly the same as the fixed version in your tree.

Thanks,

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Python extension commands in git - request for policy change

[Krzysztof Mazur]

On Mon, Nov 26, 2012 at 10:40:00AM +0530, Sitaram Chamarty wrote:
 On Mon, Nov 26, 2012 at 4:17 AM, Eric S. Raymond e...@thyrsus.com wrote:
  Krzysztof Mazur krzys...@podlesie.net:
  What about embedded systems? git is also useful there. C and shell is
  everywhere, python is not.
 
  Supposing this is true (and I question it with regard to shell) if you
  tell me how you live without gitk and the Perl pieces I'll play that
  right back at you as your answer.
 
 gitk is unlikely to be used on an embedded system, the perl pieces more so.

Currently even perl is used only for few very high level commands that
are not really needed there. I think that python is ok for pieces
that use perl now, but I think that it shouldn't be used for
basic porcelain commands. I also don't think that we should prefer
python over other languages and especially I don't think that some
existing code should be rewritten to python.

Even if python is really better, I think that the natural migration is
much better.

 
 I have never understood why people complain about readability in perl.
  Just because it uses the ascii charset a bit more?  You expect a
 mathematician or indeed any scientist to use special symbols to mean
 special things, why not programmers?
 

Because some perl programmers really create write-only code, but the
maintainer can just reject that code. It's not the language that
create non-readable code, but the programmer. I think that the perl
code in git is readable, at least is parts I've seen.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] git-send-email: allow edit invalid email address

[Krzysztof Mazur]

On Mon, Nov 26, 2012 at 09:08:34AM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  In some cases the user may want to send email with Cc: line with
  email address we cannot extract. Now we allow user to extract
  such email address for us.
 
  Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
  ---
   git-send-email.perl | 9 ++---
   1 file changed, 6 insertions(+), 3 deletions(-)
 
  diff --git a/git-send-email.perl b/git-send-email.perl
  index d42dca2..9996735 100755
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
   
   sub validate_address {
  my $address = shift;
  -   if (!extract_valid_address($address)) {
  +   while (!extract_valid_address($address)) {
  print STDERR error: unable to extract a valid address from: 
  $address\n;
  -   $_ = ask(What to do with this address? ([q]uit|[d]rop): ,
  -   valid_re = qr/^(?:quit|q|drop|d)/i,
  +   $_ = ask(What to do with this address? ([q]uit|[d]rop|[e]dit): 
  ,
  +   valid_re = qr/^(?:quit|q|drop|d|edit|e)/i,
  default = 'q');
  if (/^d/i) {
  return undef;
  @@ -862,6 +862,9 @@ sub validate_address {
  cleanup_compose_files();
  exit(0);
  }
  +   $address = ask(Who should the email be sent to (if any)? ,
  +   default = ,
  +   valid_re = qr/\@.*\./, confirm_only = 1);
 
 Not having this new code inside elsif (/^e/) { } feels somewhat
 sloppy, even though it is not *too* bad.  Also do we know this

ok, I will fix that.

 function will never be used for addresses other than recipients' (I
 gave a cursory look to see what is done to the $sender and it does
 not seem to go through this function, tho)?

Yes, this function is called only from validate_address_just()
to filter @initial_to, @initial_cc, @bcc_list as early as possible,
and filter @to and @cc added in each email.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Python extension commands in git - request for policy change

[Krzysztof Mazur]

On Sat, Nov 24, 2012 at 09:44:51PM -0500, Eric S. Raymond wrote:
 
 We're behind the best-practices curve here.  The major Linux
 distributions, which have to deal with almost the same set of
 tradeoffs we do, went to Python for pretty much all glue and
 administration scripts outside /etc a decade ago, and the decision has
 served them well.
 
 That, among other things, means up-to-date versions of Python are
 ubiquitous unless we're looking at Windows - in which case Perl and
 shell actually become much bigger portability problems.  Mac OS X 
 has kept up to date, too; Lion shipped 2.7.1 and that was a major
 release back at this point.
 

What about embedded systems? git is also useful there. C and shell is
everywhere, python is not. Adding additional dependency if it's not
really needed it's not a good idea.

Also not everyone uses up-to-date systems and sometimes you just
care about some critical parts and do not touch everything else and
there is probably quote large number of systems with python  2.6.
And even when you keep your system up-to-date, there are some GNU/Linux
distros that are still supported, but does not provide recent python - for
instance PLD Ac, which I still use on some systems and will use
until the hardware dies, provides only python 2.4.6 (by the way,
important packages like git are of course quite recent there - 1.7.11.1).

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] git-send-email: allow edit invalid email address

[Krzysztof Mazur]

In some cases the user may want to send email with Cc: line with
email address we cannot extract. Now we allow user to extract
such email address for us.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d42dca2..9996735 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -851,10 +851,10 @@ sub extract_valid_address_or_die {
 
 sub validate_address {
my $address = shift;
-   if (!extract_valid_address($address)) {
+   while (!extract_valid_address($address)) {
print STDERR error: unable to extract a valid address from: 
$address\n;
-   $_ = ask(What to do with this address? ([q]uit|[d]rop): ,
-   valid_re = qr/^(?:quit|q|drop|d)/i,
+   $_ = ask(What to do with this address? ([q]uit|[d]rop|[e]dit): 
,
+   valid_re = qr/^(?:quit|q|drop|d|edit|e)/i,
default = 'q');
if (/^d/i) {
return undef;
@@ -862,6 +862,9 @@ sub validate_address {
cleanup_compose_files();
exit(0);
}
+   $address = ask(Who should the email be sent to (if any)? ,
+   default = ,
+   valid_re = qr/\@.*\./, confirm_only = 1);
}
return $address;
 }
-- 
1.8.0.393.gcc9701d

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] git-send-email: ask what to do with an invalid email address

[Krzysztof Mazur]

We used to warn about invalid emails and just drop them. Such warnings
can be unnoticed by user or noticed after sending email when we are not
giving the final sanity check [Y/n]?

Now we quit by default.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
Suggested-by: Junio C Hamano gits...@pobox.com
---
 git-send-email.perl | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5056fdc..d42dca2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -852,8 +852,16 @@ sub extract_valid_address_or_die {
 sub validate_address {
my $address = shift;
if (!extract_valid_address($address)) {
-   print STDERR W: unable to extract a valid address from: 
$address\n;
-   return undef;
+   print STDERR error: unable to extract a valid address from: 
$address\n;
+   $_ = ask(What to do with this address? ([q]uit|[d]rop): ,
+   valid_re = qr/^(?:quit|q|drop|d)/i,
+   default = 'q');
+   if (/^d/i) {
+   return undef;
+   } elsif (/^q/i) {
+   cleanup_compose_files();
+   exit(0);
+   }
}
return $address;
 }
-- 
1.8.0.393.gcc9701d

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] git-send-email: remove invalid addresses earlier

[Krzysztof Mazur]

Some addresses are passed twice to unique_email_list() and invalid addresses
may be reported twice per send_message. Now we warn about them earlier
and we also remove invalid addresses.

This also removes using of undefined values for string comparison
for invalid addresses in cc list processing.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 52 +++-
 1 file changed, 39 insertions(+), 13 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 356f99d..5056fdc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -786,9 +786,11 @@ sub expand_one_alias {
 }
 
 @initial_to = expand_aliases(@initial_to);
-@initial_to = (map { sanitize_address($_) } @initial_to);
+@initial_to = validate_address_list(sanitize_address_list(@initial_to));
 @initial_cc = expand_aliases(@initial_cc);
+@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));
 @bcclist = expand_aliases(@bcclist);
+@bcclist = validate_address_list(sanitize_address_list(@bcclist));
 
 if ($thread  !defined $initial_reply_to  $prompting) {
$initial_reply_to = ask(
@@ -839,6 +841,28 @@ sub extract_valid_address {
return undef;
 }
 
+sub extract_valid_address_or_die {
+   my $address = shift;
+   $address = extract_valid_address($address);
+   die error: unable to extract a valid address from: $address\n
+   if !$address;
+   return $address;
+}
+
+sub validate_address {
+   my $address = shift;
+   if (!extract_valid_address($address)) {
+   print STDERR W: unable to extract a valid address from: 
$address\n;
+   return undef;
+   }
+   return $address;
+}
+
+sub validate_address_list {
+   return (grep { defined $_ }
+   map { validate_address($_) } @_);
+}
+
 # Usually don't need to change anything below here.
 
 # we make a fake message id by taking the current number
@@ -955,6 +979,10 @@ sub sanitize_address {
 
 }
 
+sub sanitize_address_list {
+   return (map { sanitize_address($_) } @_);
+}
+
 # Returns the local Fully Qualified Domain Name (FQDN) if available.
 #
 # Tightly configured MTAa require that a caller sends a real DNS
@@ -1017,14 +1045,13 @@ sub maildomain {
 
 sub send_message {
my @recipients = unique_email_list(@to);
-   @cc = (grep { my $cc = extract_valid_address($_);
+   @cc = (grep { my $cc = extract_valid_address_or_die($_);
  not grep { $cc eq $_ || $_ =~ /\Q${cc}\E$/ } @recipients
}
-  map { sanitize_address($_) }
   @cc);
my $to = join (,\n\t, @recipients);
@recipients = unique_email_list(@recipients,@cc,@bcclist);
-   @recipients = (map { extract_valid_address($_) } @recipients);
+   @recipients = (map { extract_valid_address_or_die($_) } @recipients);
my $date = format_2822_time($time++);
my $gitversion = '@@GIT_VERSION@@';
if ($gitversion =~ m/..GIT_VERSION../) {
@@ -1267,7 +1294,7 @@ foreach my $t (@files) {
foreach my $addr (parse_address_line($1)) {
printf((mbox) Adding to: %s from line 
'%s'\n,
$addr, $_) unless $quiet;
-   push @to, sanitize_address($addr);
+   push @to, $addr;
}
}
elsif (/^Cc:\s+(.*)$/) {
@@ -1376,6 +1403,9 @@ foreach my $t (@files) {
($confirm =~ /^(?:auto|compose)$/  $compose  $message_num 
== 1));
$needs_confirm = inform if ($needs_confirm  $confirm_unconfigured 
 @cc);
 
+   @to = validate_address_list(sanitize_address_list(@to));
+   @cc = validate_address_list(sanitize_address_list(@cc));
+
@to = (@initial_to, @to);
@cc = (@initial_cc, @cc);
 
@@ -1431,14 +1461,10 @@ sub unique_email_list {
my @emails;
 
foreach my $entry (@_) {
-   if (my $clean = extract_valid_address($entry)) {
-   $seen{$clean} ||= 0;
-   next if $seen{$clean}++;
-   push @emails, $entry;
-   } else {
-   print STDERR W: unable to extract a valid address,
-from: $entry\n;
-   }
+   my $clean = extract_valid_address_or_die($entry))
+   $seen{$clean} ||= 0;
+   next if $seen{$clean}++;
+   push @emails, $entry;
}
return @emails;
 }
-- 
1.8.0.393.gcc9701d

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] git-send-email: fix fallback code in extract_valid_address()

[Krzysztof Mazur]

In the fallback check, used when Email::Valid is not available, the
extract_valid_address() uses $1 without checking for success of matching
regex. The $1 variable may still hold the result of previous match,
which is the address when email address was in '' or be undefined
otherwise.

Now if match fails undefined value is always returned to indicate error.
The same value is used by Email::Valid-address() in that case.

Previously 'foo@bar' address was rejected by Email::Valid and fallback,
but 'foo@bar' was rejected by Email::Valid, but accepted by fallback.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..356f99d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -831,12 +831,12 @@ sub extract_valid_address {
$address =~ s/^\s*(.*)\s*$/$1/;
if ($have_email_valid) {
return scalar Email::Valid-address($address);
-   } else {
-   # less robust/correct than the monster regexp in Email::Valid,
-   # but still does a 99% job, and one less dependency
-   $address =~ /($local_part_regexp\@$domain_regexp)/;
-   return $1;
}
+
+   # less robust/correct than the monster regexp in Email::Valid,
+   # but still does a 99% job, and one less dependency
+   return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
+   return undef;
 }
 
 # Usually don't need to change anything below here.
-- 
1.8.0.393.gcc9701d

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Topics currently in the Stalled category

[Krzysztof Mazur]

On Tue, Nov 20, 2012 at 09:46:47PM -0500, Paul Fox wrote:
 junio c hamano wrote:
   Here is a list of stalled topics I am having trouble deciding what
   to do (the default is to dismiss them around feature freeze).
 ...
   * pf/editor-ignore-sigint (2012-11-11) 5 commits
   
Avoid confusing cases where the user hits Ctrl-C while in the editor
session, not realizing git will receive the signal. Since most editors
will take over the terminal and will block SIGINT, this is not likely
to confuse anyone.
   
Some people raised issues with emacsclient, which are addressed by this
re-roll. It should probably also handle SIGQUIT, and there were a
handful of other review comments.
   
Anybody interested in moving this forward?
 
 i started this, but then jeff took it and ran with it and made it
 right.  i think the remaining changes are small -- if jeff would
 prefer, i can probably finish it.  (but i won't guarantee not to
 mess up the From: lines.  :-)
 

I'm also interested. I sometimes use :r !command in vim, so far I never
needed to use Ctrl-C, but maybe in future.

The SIGINT part seems to be finished, we need to decide what about SIGQUIT.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Failure to extra sta...@vger.kernel.org addresses

[Krzysztof Mazur]

On Tue, Nov 20, 2012 at 11:28:39AM +0100, Felipe Contreras wrote:
 On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur krzys...@podlesie.net 
 wrote:
 
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -925,8 +925,11 @@ sub quote_subject {
   sub sanitize_address {
  my ($recipient) = @_;
 
  +   my $local_part_regexp = qr/[^\s@]+/;
  +   my $domain_regexp = qr/[^.\s@]+(?:\.[^.\s@]+)+/;
  +
  # remove garbage after email address
  -   $recipient =~ s/(.*).*$/$1/;
  +   $recipient =~ s/^(.*?$local_part_regexp\@$domain_regexp).*/$1/;
 
 I don't think all that extra complexity is warranted, to me
 s/(.*?)(.*)$/$1/ is just fine.
 

Yeah, it's a little bit too complex, but s/(.*?)(.*)$/$1/
causes small regression - '' character is no longer allowed
in phrase before email address. Maybe the initial version,
that removes everything after last '' is better? In this case ''
is not allowed in garbage after email.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: don't return undefined value in extract_valid_address()

[Krzysztof Mazur]

On Tue, Nov 20, 2012 at 12:27:36PM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  In the fallback check, when Email::Valid is not available, the
  extract_valid_address() does not check for success of matching regex,
  and $1, which can be undefined, is always returned. Now if match
  fails an empty string is returned.

Maybe the last line of comment should be changed to:

fails an empty string is returned to indicate failure.

 
 That much we can read from the code, but a bigger question is why
 would it be a good thing for the callers?  Wouldn't they want to
 be able to distinguish a failure from an empty string?

In this case returning empty string does not make sense, so it's
really used to indicate failure.

 
  Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
  ---
  This fixes following warnings:
  Use of uninitialized value in string eq at ./git-send-email.perl line 1017.
  Use of uninitialized value in quotemeta at ./git-send-email.perl line 1017.
  W: unable to extract a valid address from: x a.patch
 
  when invalid email address was added by --cc-cmd,
  ./git-send-email.perl --dry-run --to a...@podlesie.net --cc-cmd=echo x 
  a.patch
 
 In other words, would we want to *hide* (not fix) the warning?
 Shouldn't we be barfing loudly and possibly erroring it out until
 the user fixes her --cc-cmd?
 

Yes, it's just to hide the warning, the error (warning in this case) it's
already correctly generated:

W: unable to extract a valid address from: x a.patch

Maybe we should change it to an error?

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Failure to extra sta...@vger.kernel.org addresses

[Krzysztof Mazur]

On Tue, Nov 20, 2012 at 08:58:20PM +0100, Andreas Schwab wrote:
 How about s/(.*?[^]*).*$/$1/?  That will still fail on foo@bar
 foo@bar, but you'll need a full rfc822 parser to handle the general
 case anyway.

That will fail also on something foo@bar.


I think it's good compromise between complexity and correctness.

Felipe, may you check, it again? This time the change is trivial.

Andreas, may I add you in Thanks-to?

Thanks,

Krzysiek

-- 8 --
Subject: [PATCH] git-send-email: remove garbage after email address

In some cases it's very useful to add some additional information
after email in Cc-list, for instance:

Cc: Stable kernel sta...@vger.kernel.org #v3.4 v3.5 v3.6

Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.

Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.

To avoid unnecessary complexity this code assumes that in phrase before
email address 'something' never exists.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 4 
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..157eabc 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
my ($recipient) = @_;
+
+   # remove garbage after email address
+   $recipient =~ s/(.*?[^]*).*$/$1/;
+
my ($recipient_name, $recipient_addr) = ($recipient =~ 
/^(.*?)\s*(.*)/);
 
if (not $recipient_name) {
-- 
1.8.0.283.gc57d856

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Failure to extra sta...@vger.kernel.org addresses

[Krzysztof Mazur]

On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:
 Hi guys,
 
 for whatever reason my git has started acting up with
 sta...@vger.kernel.org addresses. It doesn't manage to extract a valid
 adress from the string:
 
  Cc: sta...@vger.kernel.org # v3.4 v3.5 v3.6
 
 Removing the comment at the end of the line makes things work again. I
 do remember, however, seeing this working since few weeks back I sent a
 mail to stable (in fact the same one I'm using to test), so this could
 be related to some perl updates, who knows ?!?

You probably just installed Email::Valid package.

The current git-send-email works a little better and just prints an error:

W: unable to extract a valid address from: sta...@vger.kernel.org #v3.4 v3.5 
v3.6


This patch should fix the problem, now after email any garbage is
removed while extracting address.

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..bb659da 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -828,7 +828,7 @@ sub extract_valid_address {
# check for a local address:
return $address if ($address =~ /^($local_part_regexp)$/);
 
-   $address =~ s/^\s*(.*)\s*$/$1/;
+   $address =~ s/^\s*(.*).*$/$1/;
if ($have_email_valid) {
return scalar Email::Valid-address($address);
} else {

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Failure to extra sta...@vger.kernel.org addresses

[Krzysztof Mazur]

On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
 Given that the problematic line
 
   Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y
 
 is not even a valid e-mail address, doesn't this new logic belong to
 sanitize_address() conceptually?

Yes, it's much better to do it in the sanitize_address().

Felipe, may you check it?

Krzysiek
-- 8 --
Subject: [PATCH] git-send-email: remove garbage after email address

In some cases it's very useful to add some additional information
after email in Cc-list, for instance:

Cc: Stable kernel sta...@vger.kernel.org #v3.4 v3.5 v3.6

Currently the git refuses to add such invalid email to Cc-list,
when the Email::Valid perl module is available or just uses whole line
as the email address.

Now in sanitize_address() everything after the email address is
removed, so the resulting line is correct email address and Email::Valid
validates it correctly.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 4 
 1 file changed, 4 insertions(+)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..9840d0a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -924,6 +924,10 @@ sub quote_subject {
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
my ($recipient) = @_;
+
+   # remove garbage after email address
+   $recipient =~ s/(.*).*$/$1/;
+
my ($recipient_name, $recipient_addr) = ($recipient =~ 
/^(.*?)\s*(.*)/);
 
if (not $recipient_name) {
-- 
1.8.0.283.gc57d856

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Failure to extra sta...@vger.kernel.org addresses

[Krzysztof Mazur]

On Mon, Nov 19, 2012 at 04:00:09PM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:
  Given that the problematic line
  
 Stable Kernel Maintainance Track sta...@vger.kernel.org # vX.Y
  
  is not even a valid e-mail address, doesn't this new logic belong to
  sanitize_address() conceptually?
 
  Yes, it's much better to do it in the sanitize_address().
 
 Note that I did not check that all the addresses that are handled by
 extract-valid-address came through sanitize-address function, so

Before sending that patch, I checked that and tested with and without
Email::Valid.

 unlike your original patch, this change alone may still pass some
 garbage to Email::Valid-address().  I tend to think that is a
 progress; we should make sure all the addresses are sanitized before
 using them for sending messages out.

I will try to check that.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Failure to extra sta...@vger.kernel.org addresses

[Krzysztof Mazur]

On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur krzys...@podlesie.net 
  wrote:
 
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -924,6 +924,10 @@ sub quote_subject {
   # use the simplest quoting being able to handle the recipient
   sub sanitize_address {
  my ($recipient) = @_;
  +
  +   # remove garbage after email address
  +   $recipient =~ s/(.*).*$/$1/;
  +
 
  Looks fine, but I would do s/(.*?)(.*)$/$1/, so that 'test
  f...@bar.com #comment' gets the second comment removed.
 
 Yeah, but do you need to capture the second group?  IOW, like
 s/(.*?).*$/$1/ perhaps?

I also thought about removing everything after first , but I will
not work for addresses like:

Cc: foo  sta...@vger.kernel.org #v3.4 v3.5 v3.6

What about:

$recipient =~ s/(.*[^@]*@[^]]*).*$/$1/;

or even

diff --git a/git-send-email.perl b/git-send-email.perl
index 9840d0a..b988c57 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -925,8 +925,11 @@ sub quote_subject {
 sub sanitize_address {
my ($recipient) = @_;
 
+   my $local_part_regexp = qr/[^\s@]+/;
+   my $domain_regexp = qr/[^.\s@]+(?:\.[^.\s@]+)+/;
+
# remove garbage after email address
-   $recipient =~ s/(.*).*$/$1/;
+   $recipient =~ s/(.*$local_part_regexp\@$domain_regexp).*$/$1/;
 
my ($recipient_name, $recipient_addr) = ($recipient =~ 
/^(.*?)\s*(.*)/);

which uses regex used by 99% accurate version of extract_valid_address().

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-reset man page

[Krzysztof Mazur]

On Sun, Nov 18, 2012 at 11:55:09AM -0500, Drew Northup wrote:
 
  So we should always use path for exact path, and pathspec for
  pathspecs patterns as defined in gitglossary. I think it's better
  to avoid paths and always use path... or pathspec...
 
 I suspect that the only reason why the differentiation between
 path and paths happened is because there may be some places
 where it was seen that a _list of paths_ was acceptable (which isn't a
 pathspec, as it isn't a search expression) and other places where

paths is usually used for a list of pathspec, not just a list of path.

 _only_ a single path was acceptable. Should that fail to be the case
 then there would be a good argument for changing the affected
 instances of paths to path in the documentation. (I know of no
 other good way to pluralize path myself.)
 

I think it's best to just add ...:

path for single exact path,
path... for a list of exact paths,
pathspec for single pathspec,
pathspec... for a list of pathspecs.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] ignore SIGINT while editor runs

[Krzysztof Mazur]

On Sun, Nov 11, 2012 at 03:24:19PM -0500, Paul Fox wrote:
 krzysztof wrote:
   Looks ok, but what about SIGQUIT? Some editors like GNU ed (0.4 and 1.6)
   ignore SIGQUIT, and after SIGQUIT git dies, but editor is still running.
   After pressing any key ed receives -EIO and prints stdin: Input/output
   error. GNU ed 1.6 then exits, but ed 0.4 prints this error forever.
   Maybe git should kill the editor in such case?
 
 there's certainly lots of precedent for treating SIGINT and SIGQUIT
 the same.  but there's also some merit to saying that if the user
 knows to send SIGQUIT instead of SIGINT, they may well have a reason. 
 (after all, if we always treat them the same, there's no point in
 having both.)

That's why I'm proposing in case of SIGQUIT just killing the editor
(SIGTERM is sufficient for ed).

So git will ignore SIGINT, but die on SIGQUIT (and kill editor
that ignores SIGQUIT).

 
 the em editor (linus' microemacs) behaves as you describe ed 0.4 does,
 except without the error message -- it just spins silently getting EIO
 from reading stdin.  i think em needs to be fixed, and it sounds like
 GNU ed already has been.  (unless i misunderstand the relationship of
 0.4 and 1.6.)

Yes, the version 1.6 is fixed, it just prints an error once and exits.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-reset man page

[Krzysztof Mazur]

On Sat, Nov 10, 2012 at 10:55:13AM +0100, Angelo Borsotti wrote:
 Hi
 
 the man page of git-reset, synopsys, does not allow for an
 argumentless call, and the description does not tell either what is
 the meaning of it.

This issue was already reported by Bojan Petrović:
http://thread.gmane.org/gmane.comp.version-control.git/208505

and fixed in commit d505865be5c7d72abb74318940e8c4c52aa0db5f
(doc: git-reset: make mode optional) in master branch.

git reset is equivalent to git reset --mixed.

 Suggested changes:
 
 first line of synopsis:
 
   gitt reset [-q] [commit] [ [--] pathspec ...]
 
 Description: append to the end of the first paragraph:
 
  If no pathspecs are specified, all the index entries are reset.
 
 I would suggest to change paths with pathspec in all the man page
 because paths in the glossary are called pathspecs.
 

The paths issue seems to be bigger - path, paths and pathspec
are mixed in whole manual:

$ cat Documentation/*.txt | grep -o 'path[^]*' | sort | uniq -c |
sort -n -r
125 path
 17 paths
 10 pathspec
  2 pathtemplate
  2 path-pattern
  1 path_to
  1 path_from

In commands it's even worse:
$ cat builtin/*.c | grep -o 'path[^]*' | sort | uniq -c | sort -n -r
 14 path
 15 paths

Note: path is not always used for pathspec.

In git-checkout manual in synopsis paths is used, but in description
pathspec.


Maybe we should just add that paths is an shortcut for pathspec
and fix places where paths and pathspec are mixed or path is used as
pathspec.


Thanks,

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-reset man page

[Krzysztof Mazur]

On Sat, Nov 10, 2012 at 12:02:12PM -0800, Junio C Hamano wrote:
 Krzysztof Mazur krzys...@podlesie.net writes:
 
  Maybe we should just add that paths is an shortcut for pathspec
  and fix places where paths and pathspec are mixed or path is used as
  pathspec.
 
 We should unify uses of paths and path (the former should be
 path... or something).

Currently in most cases paths... is used ;)

 
 Some places you need to give exact path (iow, these places you
 cannot use pathspec), while most other places pathspec
 (i.e. matching pattern) is accepted.

I know, thats why I added a note that path is not always used for pathspec.

 
 The manual correctly updated will most likely to use both path and
 pathspec appropriately.

So we should always use path for exact path, and pathspec for
pathspecs patterns as defined in gitglossary. I think it's better
to avoid paths and always use path... or pathspec

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] launch_editor: ignore SIGINT while the editor has control

[Krzysztof Mazur]

On Wed, Nov 07, 2012 at 02:16:52PM -0500, Paul Fox wrote:
 the user's editor likely catches SIGINT (ctrl-C).  but if the user
 spawns a command from the editor and uses ctrl-C to kill that command,
 the SIGINT will likely also kill git itself.  (depending on the
 editor, this can leave the terminal in an unusable state.)
 
 Signed-off-by: Paul Fox p...@foxharp.boston.ma.us
 
  editor.c |6 +-
  1 files changed, 5 insertions(+), 1 deletions(-)
 
 diff --git a/editor.c b/editor.c
 index d834003..775f22d 100644
 --- a/editor.c
 +++ b/editor.c
 @@ -37,8 +37,12 @@ int launch_editor(const char *path, struct strbuf *buffer, 
 const char *const *en
  
   if (strcmp(editor, :)) {
   const char *args[] = { editor, path, NULL };
 + int ret;
  
 - if (run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, env))
 + sigchain_push(SIGINT, SIG_IGN);
 + ret = run_command_v_opt_cd_env(args, RUN_USING_SHELL, NULL, 
 env);
 + sigchain_pop(SIGINT);
 + if (ret)
   return error(There was a problem with the editor 
 '%s'.,
   editor);
   }

Looks and works good, except for warnings:

editor.c: In function 'launch_editor':
editor.c:42:3: warning: implicit declaration of function 'sigchain_push' 
[-Wimplicit-function-declaration]
editor.c:44:3: warning: implicit declaration of function 'sigchain_pop' 
[-Wimplicit-function-declaration]

sigchain.h should be included, something like:

diff --git a/editor.c b/editor.c
index 775f22d..3ca361b 100644
--- a/editor.c
+++ b/editor.c
@@ -1,6 +1,7 @@
 #include cache.h
 #include strbuf.h
 #include run-command.h
+#include sigchain.h
 
 #ifndef DEFAULT_EDITOR
 #define DEFAULT_EDITOR vi

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Mistake in git-reset documentation

[Krzysztof Mazur]

On Sat, Oct 27, 2012 at 01:21:18PM +0200, Bojan Petrović wrote:
 None of the three forms of git-reset accept: git reset which is the
 equivalent of git reset -mixed.
 
 Square brackets should be used instead of parentheses for --soft |
 --mixed | --hard | --merge | --keep.
 
 Bojan

Square brackets are also missing in 'git reset' --mode [commit].

Bojan, do you want to add a Reported-by line with your name?

Krzysiek

-- 8 --
Subject: [PATCH] doc: git-reset: make --mode optional

The git-reset's --mode is an optional argument, however it was
documented as required.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 Documentation/git-reset.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 117e374..1f95292 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [-q] [commit] [--] paths...
 'git reset' (--patch | -p) [commit] [--] [paths...]
-'git reset' (--soft | --mixed | --hard | --merge | --keep) [-q] [commit]
+'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
 
 DESCRIPTION
 ---
@@ -43,7 +43,7 @@ This means that `git reset -p` is the opposite of `git add 
-p`, i.e.
 you can use it to selectively reset hunks. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
 
-'git reset' --mode [commit]::
+'git reset' [--mode] [commit]::
This form resets the current branch head to commit and
possibly updates the index (resetting it to the tree of commit) and
the working tree depending on mode, which
-- 
1.8.0.46.gd11dae0

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Mistake in git-reset documentation

[Krzysztof Mazur]

On Sun, Oct 28, 2012 at 09:46:35AM -0400, Jeff King wrote:
 On Sun, Oct 28, 2012 at 02:39:49PM +0100, Andreas Schwab wrote:
 
  Jeff King p...@peff.net writes:
  
   On Sun, Oct 28, 2012 at 09:36:10AM +0100, Krzysztof Mazur wrote:
  
DESCRIPTION
---
   @@ -43,7 +43,7 @@ This means that `git reset -p` is the opposite of `git 
   add -p`, i.e.
you can use it to selectively reset hunks. See the ``Interactive Mode''
section of linkgit:git-add[1] to learn how to operate the `--patch` 
   mode.

   -'git reset' --mode [commit]::
   +'git reset' [--mode] [commit]::
This form resets the current branch head to commit and
possibly updates the index (resetting it to the tree of 
   commit) and
the working tree depending on mode, which
  
   Should we say something like if --mode is omitted, defaults to
   --mixed?
  
  Under --mixed it already says This is the default action, though.
 
 I know, but that is somewhat buried for somebody who is seeing that the
 --mode bit is optional and wondering what it means to omit it.
 

The --mixed mode is also described as second mode, and saying that --mixed
is default earlier may save some time wasted on reading --soft
description.

There is also small inconsequence in what mode is, just mixed or
--mixed.

Krzysiek

-- 8 --
Subject: [PATCH] doc: git-reset: make mode optional

The git-reset's mode is an optional argument, however it was
documented as required.

The mode is documented as one of: --soft, --mixed, --hard, --merge
or --keep, so mode should be used instead of --mode.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 Documentation/git-reset.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 117e374..978d8da 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [-q] [commit] [--] paths...
 'git reset' (--patch | -p) [commit] [--] [paths...]
-'git reset' (--soft | --mixed | --hard | --merge | --keep) [-q] [commit]
+'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
 
 DESCRIPTION
 ---
@@ -43,11 +43,11 @@ This means that `git reset -p` is the opposite of `git add 
-p`, i.e.
 you can use it to selectively reset hunks. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
 
-'git reset' --mode [commit]::
+'git reset' [mode] [commit]::
This form resets the current branch head to commit and
possibly updates the index (resetting it to the tree of commit) and
-   the working tree depending on mode, which
-   must be one of the following:
+   the working tree depending on mode. If mode is omitted,
+   defaults to --mixed. The mode must be one of the following:
 +
 --
 --soft::
-- 
1.8.0.47.g5b520ba
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: skip RFC2047 quoting for ASCII subjects

[Krzysztof Mazur]

On Thu, Oct 25, 2012 at 05:01:49AM -0400, Jeff King wrote:
 
 Hmm. What is this patch on top of? It looks like it is on top of your
 original patch, but when I tried it on top of that, it does not apply
 either, and the index lines in the patch do not mention a sha1 that I do
 not have.

Sorry, it's against km/send-email-compose-encoding (or current next)
+ git-send-email: use compose-encoding for Subject.

 
 Do you mind re-rolling a final 2-patch series with:
 
   1. Your original patch and this one squashed together, with an
  appropriate commit message.

I think that it's better to do refactoring and fix for ASCII in separate
patches. Maybe we should reverse order of first two patches. This first
will do refactoring and the second will just replace quote_rfc2047()
with quote_subject() in broken encoding case and add test
for this problem.

 
   2. The second quote when we see '=?' patch.
 
 Thanks.
 
 -Peff

ok, I will resend the final series.

I need also to fix git-send-email: use compose-encoding for Subject
patch. Now it's depends both on this series and
km/send-email-compose-encoding branch.

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: skip RFC2047 quoting for ASCII subjects

[Krzysztof Mazur]

On Thu, Oct 25, 2012 at 06:08:54AM -0400, Jeff King wrote:
 
 Ah, never mind. I missed your earlier use compose-encoding for
 Subject. I've queued it and all of the follow-ons onto the
 km/send-email-compose-encoding topic.
 

thanks, what about the problem with whitespaces in quote_subject patch?

Krzysiek
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-send-email: skip RFC2047 quoting for ASCII subjects

[Krzysztof Mazur]

The git-send-email always use RFC2047 subject quoting for files
with broken encoding - non-ASCII files without Content-Transfer-Encoding,
even for ASCII subjects. Now for ASCII subjects the RFC2047 quoting will be
skipped.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl   |  3 ++-
 t/t9001-send-email.sh | 17 +
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index adcb4e3..efeae4c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1327,7 +1327,8 @@ foreach my $t (@files) {
$body_encoding = $auto_8bit_encoding;
}
 
-   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject)) {
+   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject) 
+   ($subject =~ /[^[:ascii:]]/)) {
$subject = quote_rfc2047($subject, $auto_8bit_encoding);
}
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 89fceda..6c6af7d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1143,6 +1143,23 @@ EOF
 '
 
 test_expect_success $PREREQ 'setup expect' '
+cat expected EOF
+Subject: subject goes here
+EOF
+'
+
+test_expect_success $PREREQ 'ASCII subject is not RFC2047 quoted' '
+   clean_fake_sendmail 
+   echo bogus |
+   git send-email --from=aut...@example.com --to=nob...@example.com \
+   --smtp-server=$(pwd)/fake.sendmail \
+   --8bit-encoding=UTF-8 \
+   email-using-8bit stdout 
+   grep Subject msgtxt1 actual 
+   test_cmp expected actual
+'
+
+test_expect_success $PREREQ 'setup expect' '
 cat content-type-decl EOF
 MIME-Version: 1.0
 Content-Type: text/plain; charset=UTF-8
-- 
1.8.0.3.gf4c35fc

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: skip RFC2047 quoting for ASCII subjects

[Krzysztof Mazur]

On Wed, Oct 24, 2012 at 04:46:36AM -0400, Jeff King wrote:
 On Wed, Oct 24, 2012 at 10:03:35AM +0200, Krzysztof Mazur wrote:
 
  The git-send-email always use RFC2047 subject quoting for files
  with broken encoding - non-ASCII files without Content-Transfer-Encoding,
  even for ASCII subjects. Now for ASCII subjects the RFC2047 quoting will be
  skipped.
  [...]
  -   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject)) {
  +   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject) 
  +   ($subject =~ /[^[:ascii:]]/)) {
 
 Is that test sufficient? We would also need to encode if it has rfc2047
 specials, no?

For Subject this should be sufficient. According to RFC822 after
Subject: we have text token,

--- from RFC822 ---
 /  Subject   :  *text
--- from RFC822 ---

and text is defined as:

--- from RFC822 ---
 text=  any CHAR, including bare; = atoms, specials,
 CR  bare LF, but NOT   ;  comments and
 including CRLF ;  quoted-strings are
 ;  NOT recognized.
--- from RFC822 ---

so only CRLF is not allowed in Subject.


So the problem only exists for broken RFC2047-like texts, but I think
it's ok to just pass such subjects, in most cases the Subject comes
from already formatted patch file. I think that we just want to fix Subjects
without specified encoding here.


In most cases, when git-send-email is used for patches generated
by git format-patch we just don't want to corrupt Subject. The
git format-patch generates broken patches when commit message
uses only ASCII characters and patch contains some non-ASCII characters.
In this case original git-send-email, without this patch, adds RFC2047
quotation for pure ASCII Subject.

 
 It looks like we use the same regex elsewhere. Maybe this would be a
 good chance to abstract out a needs_rfc2047_quoting while we are in the
 area?

It's a good idea, however rules are different for Subject and addresses
(sanitize_address).

I think we can go even further, we can just add quote_subject(),
which performs this test and calls quote_rfc2047() if necessary.
I'm sending bellow patch that does that.

Krzysiek
-- 
From a1e6eef831275485ec1555d94ff0d9aac852dd12 Mon Sep 17 00:00:00 2001
From: Krzysztof Mazur krzys...@podlesie.net
Date: Wed, 24 Oct 2012 19:08:57 +0200
Subject: [PATCH] git-send-email: introduce quote_subject()

The quote_rfc2047() always adds RFC2047 quoting and to avoid quoting ASCII
subjects, before calling quote_rfc2047() subject must be tested for non-ASCII
characters. To avoid this new quote_subject() function is introduced.
The quote_subject() performs this test and calls quote_rfc2047() only if
necessary.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index efeae4c..e9aec8d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -657,9 +657,7 @@ EOT
$initial_subject = $1;
my $subject = $initial_subject;
$_ = Subject:  .
-   ($subject =~ /[^[:ascii:]]/ ?
-quote_rfc2047($subject, $compose_encoding) :
-$subject) .
+   quote_subject($subject, $compose_encoding) .
\n;
} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
$initial_reply_to = $1;
@@ -907,6 +905,22 @@ sub is_rfc2047_quoted {
$s =~ m/^(?:[[:ascii:]]*|=\?$token\?$token\?$encoded_text\?=)$/o;
 }
 
+sub subject_needs_rfc2047_quoting {
+   my $s = shift;
+
+   return !is_rfc2047_quoted($s)  ($s =~ /[^[:ascii:]]/);
+}
+
+sub quote_subject {
+   local $subject = shift;
+   my $encoding = shift || 'UTF-8';
+
+   if (subject_needs_rfc2047_quoting($subject)) {
+   return quote_rfc2047($subject, $encoding);
+   }
+   return $subject;
+}
+
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
my ($recipient) = @_;
@@ -1327,9 +1341,8 @@ foreach my $t (@files) {
$body_encoding = $auto_8bit_encoding;
}
 
-   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject) 
-   ($subject =~ /[^[:ascii:]]/)) {
-   $subject = quote_rfc2047($subject, $auto_8bit_encoding);
+   if ($broken_encoding{$t}) {
+   $subject = quote_subject($subject, $auto_8bit_encoding);
}
 
if (defined $author and $author ne $sender) {
-- 
1.8.0.3.gf4c35fc

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-send-email: skip RFC2047 quoting for ASCII subjects

[Krzysztof Mazur]

On Wed, Oct 24, 2012 at 03:25:30PM -0400, Jeff King wrote:
 Right, but I was specifically worried about raw =?, which is only an
 issue due to rfc2047 itself.
 
 However, reading the patch again, we are already checking for that with
 is_rfc2047_quoted. It might miss the case where we have =? but not the
 rest of a valid encoded word, but any compliant parser should recognize
 that and leave it be.
 
 So I think your original patch is actually correct.
 
 [...]
 We have a possibly already-encoded header, and we would want to avoid
 double-encoding it.
 
 In the first case, the wants quoting logic should be:
 
   is_rfc2047_quoted($subject) || /[^[:ascii:]]/
 
 and in the latter case it would be:
 
   !is_rfc2047_quoted($subject)  /^[:ascii:]]/
 

ok, I'm sending a version that just adds quote_subject() without
changing any logic, so now we still have in first case:

 /[^[:ascii:]]/

and in the latter case:
 
 !is_rfc2047_quoted($subject)  /^[:ascii:]]/


In the next patch I will just add matching for =? in 
subject_needs_rfc2047_quoting() and we will have:

   /=?/ || /[^[:ascii:]]/

and in the latter case:
 
   !is_rfc2047_quoted($subject)  (/=\?/ || /^[:ascii:]]/)

This will also add quoting for any rfc2047 quoted subject or any
other rfc2047-like subject, as you suggested.

Krzysiek
-- 
From a70c5385f9b4da69a8ce00a1448f87f63bbd500d Mon Sep 17 00:00:00 2001
From: Krzysztof Mazur krzys...@podlesie.net
Date: Wed, 24 Oct 2012 22:46:00 +0200
Subject: [PATCH] git-send-email: introduce quote_subject()

The quote_rfc2047() always adds RFC2047 quoting and to avoid quoting ASCII
subjects, before calling quote_rfc2047() subject must be tested for non-ASCII
characters. To avoid this new quote_subject() function is introduced.
The quote_subject() performs this test and calls quote_rfc2047() only if
necessary.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index efeae4c..eb1b876 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -657,9 +657,7 @@ EOT
$initial_subject = $1;
my $subject = $initial_subject;
$_ = Subject:  .
-   ($subject =~ /[^[:ascii:]]/ ?
-quote_rfc2047($subject, $compose_encoding) :
-$subject) .
+   quote_subject($subject, $compose_encoding) .
\n;
} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
$initial_reply_to = $1;
@@ -907,6 +905,22 @@ sub is_rfc2047_quoted {
$s =~ m/^(?:[[:ascii:]]*|=\?$token\?$token\?$encoded_text\?=)$/o;
 }
 
+sub subject_needs_rfc2047_quoting {
+   my $s = shift;
+
+   return ($s =~ /[^[:ascii:]]/);
+}
+
+sub quote_subject {
+   local $subject = shift;
+   my $encoding = shift || 'UTF-8';
+
+   if (subject_needs_rfc2047_quoting($subject)) {
+   return quote_rfc2047($subject, $encoding);
+   }
+   return $subject;
+}
+
 # use the simplest quoting being able to handle the recipient
 sub sanitize_address {
my ($recipient) = @_;
@@ -1327,9 +1341,8 @@ foreach my $t (@files) {
$body_encoding = $auto_8bit_encoding;
}
 
-   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject) 
-   ($subject =~ /[^[:ascii:]]/)) {
-   $subject = quote_rfc2047($subject, $auto_8bit_encoding);
+   if ($broken_encoding{$t}  !is_rfc2047_quoted($subject)) {
+   $subject = quote_subject($subject, $auto_8bit_encoding);
}
 
if (defined $author and $author ne $sender) {
-- 
1.8.0.4.ge8ddce6
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-send-email: add rfc2047 quoting for =?

[Krzysztof Mazur]

For raw subjects rfc2047 quoting is needed not only for non-ASCII characters,
but also for any possible rfc2047 in it.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
Oops, this ugly Subject was generated by git format-patch (both 1.8.0
and km/send-email-compose-encoding).

 git-send-email.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index eb1b876..cfd20fa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -908,7 +908,7 @@ sub is_rfc2047_quoted {
 sub subject_needs_rfc2047_quoting {
my $s = shift;
 
-   return ($s =~ /[^[:ascii:]]/);
+   return ($s =~ /[^[:ascii:]]/) || ($s =~ /=\?/);
 }
 
 sub quote_subject {
-- 
1.8.0.rc0.30.g72615bf

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-send-email: use compose-encoding for Subject

[Krzysztof Mazur]

The commit git-send-email: introduce compose-encoding introduced
the compose-encoding option to specify the introduction email encoding
(--compose option), but the email Subject encoding was still hardcoded
to UTF-8.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
Patch against km/send-email-compose-encoding
(commit 62e0069056ed11294c29bae25df69b6518f6339e). Cleanly applies to current
next (commit 291341ca77d902dc76e204a3fc498a155f0ab75d)

 git-send-email.perl   |  8 
 t/t9001-send-email.sh | 14 ++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 107e814..adcb4e3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -636,15 +636,15 @@ EOT
my $need_8bit_cte = file_has_nonascii($compose_filename);
my $in_body = 0;
my $summary_empty = 1;
+   if (!defined $compose_encoding) {
+   $compose_encoding = UTF-8;
+   }
while($c) {
next if m/^GIT:/;
if ($in_body) {
$summary_empty = 0 unless (/^\n$/);
} elsif (/^\n$/) {
$in_body = 1;
-   if (!defined $compose_encoding) {
-   $compose_encoding = UTF-8;
-   }
if ($need_8bit_cte) {
print $c2 MIME-Version: 1.0\n,
 Content-Type: text/plain; ,
@@ -658,7 +658,7 @@ EOT
my $subject = $initial_subject;
$_ = Subject:  .
($subject =~ /[^[:ascii:]]/ ?
-quote_rfc2047($subject) :
+quote_rfc2047($subject, $compose_encoding) :
 $subject) .
\n;
} elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 265ae04..89fceda 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -909,6 +909,20 @@ test_expect_success $PREREQ '--compose-encoding overrides 
sendemail.composeencod
grep ^Content-Type: text/plain; charset=iso-8859-2 msgtxt1
 '
 
+test_expect_success $PREREQ '--compose-encoding adds correct MIME for subject' 
'
+   clean_fake_sendmail 
+ GIT_EDITOR=\$(pwd)/fake-editor\ \
+ git send-email \
+ --compose-encoding iso-8859-2 \
+ --compose --subject utf8-sübjëct \
+ --from=Example nob...@example.com \
+ --to=nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ $patches 
+   grep ^fake edit msgtxt1 
+   grep ^Subject: =?iso-8859-2?q?utf8-s=C3=BCbj=C3=ABct?= msgtxt1
+'
+
 test_expect_success $PREREQ 'detects ambiguous reference/file conflict' '
echo master  master 
git add master 
-- 
1.8.0.2.g35080e9

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] git-send-email: use locale encoding for compose

[Krzysztof Mazur]

The introduction email (--compose option) use UTF-8 as default encoding.
The current locale encoding is much better default value.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 git-send-email.perl | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 107e814..139bb35 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -590,6 +590,16 @@ sub get_patch_subject {
die No subject line in $fn ?;
 }
 
+sub locale_encoding {
+   my $encoding = UTF-8;
+   eval {
+   require I18N::Langinfo;
+   I18N::Langinfo-import(qw(langinfo CODESET));
+   $encoding = langinfo(CODESET());
+   };
+   return $encoding;
+}
+
 if ($compose) {
# Note that this does not need to be secure, but we will make a small
# effort to have it be unique
@@ -643,7 +653,7 @@ EOT
} elsif (/^\n$/) {
$in_body = 1;
if (!defined $compose_encoding) {
-   $compose_encoding = UTF-8;
+   $compose_encoding = locale_encoding();
}
if ($need_8bit_cte) {
print $c2 MIME-Version: 1.0\n,
-- 
1.7.12.2.2.g1c3c581

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] git-send-email: introduce compose-encoding

[Krzysztof Mazur]

The introduction email (--compose option) have encoding hardcoded to
UTF-8, but invoked editor may not use UTF-8 encoding.
The encoding used by patches can be changed by the 8bit-encoding
option, but this option does not have effect on introduction email
and equivalent for introduction email is missing.

Added compose-encoding command line option and sendemail.composeencoding
configuration option specify encoding of introduction email.

Signed-off-by: Krzysztof Mazur krzys...@podlesie.net
---
 Documentation/git-send-email.txt | 5 +
 git-send-email.perl  | 9 -
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3241170..9f09e92 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -126,6 +126,11 @@ The --to option must be repeated for each user you want on 
the to list.
 +
 Note that no attempts whatsoever are made to validate the encoding.
 
+--compose-encoding=encoding::
+   Specify encoding of compose message. Default is the value of the
+   'sendemail.composeencoding'; if that is unspecified, UTF-8 is assumed.
++
+
 
 Sending
 ~~~
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..107e814 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -56,6 +56,7 @@ git send-email [options] file | directory | rev-list options 

 --in-reply-to   str  * Email In-Reply-To:
 --annotate * Review each patch that will be sent in an 
editor.
 --compose  * Open an editor for introduction.
+--compose-encoding  str  * Encoding to assume for introduction.
 --8bit-encoding str  * Encoding to assume 8bit mails if 
undeclared
 
   Sending:
@@ -198,6 +199,7 @@ my ($identity, $aliasfiletype, @alias_files, $smtp_domain);
 my ($validate, $confirm);
 my (@suppress_cc);
 my ($auto_8bit_encoding);
+my ($compose_encoding);
 
 my ($debug_net_smtp) = 0;  # Net::SMTP, see send_message()
 
@@ -231,6 +233,7 @@ my %config_settings = (
 confirm   = \$confirm,
 from = \$sender,
 assume8bitencoding = \$auto_8bit_encoding,
+composeencoding = \$compose_encoding,
 );
 
 my %config_path_settings = (
@@ -315,6 +318,7 @@ my $rc = GetOptions(h = \$help,
validate! = \$validate,
format-patch! = \$format_patch,
8bit-encoding=s = \$auto_8bit_encoding,
+   compose-encoding=s = \$compose_encoding,
force = \$force,
 );
 
@@ -638,10 +642,13 @@ EOT
$summary_empty = 0 unless (/^\n$/);
} elsif (/^\n$/) {
$in_body = 1;
+   if (!defined $compose_encoding) {
+   $compose_encoding = UTF-8;
+   }
if ($need_8bit_cte) {
print $c2 MIME-Version: 1.0\n,
 Content-Type: text/plain; ,
-  charset=UTF-8\n,
+  charset=$compose_encoding\n,
 Content-Transfer-Encoding: 8bit\n;
}
} elsif (/^MIME-Version:/i) {
-- 
1.7.12.2.2.g1c3c581

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html