Re: [PATCH] am: invoke perl's strftime in C locale

2013-01-15 Thread Jeff King 
On Tue, Jan 15, 2013 at 12:59:33AM +0400, Dmitry V. Levin wrote:

 diff --git a/git-am.sh b/git-am.sh
 index c682d34..64b88e4 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -334,7 +334,7 @@ split_patches () {
   # Since we cannot guarantee that the commit message is 
 in
   # git-friendly format, we put no Subject: line and just 
 consume
   # all of the message as the body
 - perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
 + LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN { 
 $subject = 0 }
   if ($subject) { print ; }
   elsif (/^\# User /) { s/\# User/From:/ ; print 
 ; }
   elsif (/^\# Date /) {

This puts all of perl into the C locale, which would mean error messages
from perl would be in English rather than the user's language. It
probably isn't a big deal, because that snippet of perl is short and not
likely to produce problems, but I wonder how hard it would be to set the
locale just for the strftime call.

-Peff
--
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] am: invoke perl's strftime in C locale

2013-01-15 Thread Antoine Pelisse 
 This puts all of perl into the C locale, which would mean error messages
 from perl would be in English rather than the user's language. It
 probably isn't a big deal, because that snippet of perl is short and not
 likely to produce problems, but I wonder how hard it would be to set the
 locale just for the strftime call.

Maybe just setting LC_TIME to C would do ...

From locale(7) man page:

   LC_TIME
  changes  the behavior of the strftime(3) function to
display the current time in a locally acceptable form; for
  example, most of Europe uses a 24-hour clock versus the
12-hour clock used in the United States.
--
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] am: invoke perl's strftime in C locale

2013-01-15 Thread Dmitry V. Levin 
On Tue, Jan 15, 2013 at 08:50:59AM -0800, Jeff King wrote:
 On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
 
   This puts all of perl into the C locale, which would mean error messages
   from perl would be in English rather than the user's language. It
   probably isn't a big deal, because that snippet of perl is short and not
   likely to produce problems, but I wonder how hard it would be to set the
   locale just for the strftime call.
  
  Maybe just setting LC_TIME to C would do ...
 
 Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
 fix the problem for you?

Just setting LC_TIME environment variable instead of LC_ALL would end up
with unreliable solution because LC_ALL has the highest priority.

If keeping error messages from perl has the utmost importance, it could be
achieved by
-   perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
+   perl -M'POSIX qw(strftime :locale_h)' -ne '
+   BEGIN { setlocale(LC_TIME, C); $subject = 0 }
but the little perl helper script we are talking about hardly worths so
much efforts.


-- 
ldv


pgp6E64QU8Sck.pgp
Description: PGP signature

Re: [PATCH] am: invoke perl's strftime in C locale

2013-01-15 Thread Junio C Hamano 
Dmitry V. Levin l...@altlinux.org writes:

 On Tue, Jan 15, 2013 at 08:50:59AM -0800, Jeff King wrote:
 On Tue, Jan 15, 2013 at 05:42:12PM +0100, Antoine Pelisse wrote:
 
   This puts all of perl into the C locale, which would mean error messages
   from perl would be in English rather than the user's language. It
   probably isn't a big deal, because that snippet of perl is short and not
   likely to produce problems, but I wonder how hard it would be to set the
   locale just for the strftime call.
  
  Maybe just setting LC_TIME to C would do ...
 
 Yeah, that is a nice simple solution. Dmitry, does just setting LC_TIME
 fix the problem for you?

 Just setting LC_TIME environment variable instead of LC_ALL would end up
 with unreliable solution because LC_ALL has the highest priority.

 If keeping error messages from perl has the utmost importance, it could be
 achieved by
 - perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
 + perl -M'POSIX qw(strftime :locale_h)' -ne '
 + BEGIN { setlocale(LC_TIME, C); $subject = 0 }
 but the little perl helper script we are talking about hardly worths so
 much efforts.

Yeah I agree that this is not worth it, I would think.
--
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] am: invoke perl's strftime in C locale

2013-01-14 Thread Junio C Hamano 
Dmitry V. Levin l...@altlinux.org writes:

 This fixes hg patch format support for locales other than C and en_*,
 see https://bugzilla.altlinux.org/show_bug.cgi?id=28248

 Signed-off-by: Dmitry V. Levin l...@altlinux.org
 ---

Thanks.

The reference URL is not very friendly, and you should be able to
state it here on a single line in English instead, I think.

The patch looks correct, though.

  git-am.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/git-am.sh b/git-am.sh
 index c682d34..64b88e4 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -334,7 +334,7 @@ split_patches () {
   # Since we cannot guarantee that the commit message is 
 in
   # git-friendly format, we put no Subject: line and just 
 consume
   # all of the message as the body
 - perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 }
 + LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN { 
 $subject = 0 }
   if ($subject) { print ; }
   elsif (/^\# User /) { s/\# User/From:/ ; print 
 ; }
   elsif (/^\# Date /) {
--
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