* [PATCH] t0200: "locale" may not exist
@ 2012-12-19 6:47 Junio C Hamano
2012-12-19 13:18 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2012-12-19 6:47 UTC (permalink / raw)
To: git
On systems without "locale" installed, t0200-gettext-basic.sh leaked
error messages when checking if some test locales are available.
Hide them, as they are not very useful.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/lib-gettext.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..ae8883a 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -14,12 +14,14 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
then
# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
- is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
+ is_IS_locale=$(locale -a 2>/dev/null |
+ sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
p
q
}')
# is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
- is_IS_iso_locale=$(locale -a | sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
+ is_IS_iso_locale=$(locale -a 2>/dev/null |
+ sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
p
q
}')
--
1.8.1.rc2.196.g654d69e
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] t0200: "locale" may not exist
2012-12-19 6:47 [PATCH] t0200: "locale" may not exist Junio C Hamano
@ 2012-12-19 13:18 ` Jeff King
2012-12-19 15:28 ` Junio C Hamano
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2012-12-19 13:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Tue, Dec 18, 2012 at 10:47:03PM -0800, Junio C Hamano wrote:
> On systems without "locale" installed, t0200-gettext-basic.sh leaked
> error messages when checking if some test locales are available.
> Hide them, as they are not very useful.
Obviously correct, though there is another way:
> diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
> index 0f76f6c..ae8883a 100644
> --- a/t/lib-gettext.sh
> +++ b/t/lib-gettext.sh
> @@ -14,12 +14,14 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
> if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
If we turn this line into:
test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
then people can see the error output of the setup step in verbose mode.
Annoyingly, though, it means tweaking the quoting throughout the block
to handle embedded single-quotes (if there is one feature I could take
from perl back into shell, it would be arbitrary quote delimiters).
Patch is below. I don't know if it is worth the complexity.
-Peff
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..d962c00 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -11,18 +11,17 @@ then
. "$GIT_BUILD_DIR"/git-sh-i18n
-if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
-then
+test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
- is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
+ is_IS_locale=$(locale -a | sed -n "/^is_IS\.[uU][tT][fF]-*8\$/{
p
q
- }')
+ }")
# is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
- is_IS_iso_locale=$(locale -a | sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
+ is_IS_iso_locale=$(locale -a | sed -n "/^is_IS\.[iI][sS][oO]8859-*1\$/{
p
q
- }')
+ }")
# Export them as an environment variable so the t0202/test.pl Perl
# test can use it too
@@ -37,7 +36,7 @@ then
# Exporting for t0202/test.pl
GETTEXT_LOCALE=1
export GETTEXT_LOCALE
- say "# lib-gettext: Found '$is_IS_locale' as an is_IS UTF-8 locale"
+ say "# lib-gettext: Found \"$is_IS_locale\" as an is_IS UTF-8 locale"
else
say "# lib-gettext: No is_IS UTF-8 locale available"
fi
@@ -48,8 +47,8 @@ then
# Some of the tests need the reference Icelandic locale
test_set_prereq GETTEXT_ISO_LOCALE
- say "# lib-gettext: Found '$is_IS_iso_locale' as an is_IS ISO-8859-1 locale"
+ say "# lib-gettext: Found \"$is_IS_iso_locale\" as an is_IS ISO-8859-1 locale"
else
say "# lib-gettext: No is_IS ISO-8859-1 locale available"
fi
-fi
+'
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] t0200: "locale" may not exist
2012-12-19 13:18 ` Jeff King
@ 2012-12-19 15:28 ` Junio C Hamano
2012-12-19 15:32 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2012-12-19 15:28 UTC (permalink / raw)
To: Jeff King; +Cc: git
Jeff King <peff@peff.net> writes:
> On Tue, Dec 18, 2012 at 10:47:03PM -0800, Junio C Hamano wrote:
>
>> On systems without "locale" installed, t0200-gettext-basic.sh leaked
>> error messages when checking if some test locales are available.
>> Hide them, as they are not very useful.
>
> Obviously correct, though there is another way:
>
>> diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
>> index 0f76f6c..ae8883a 100644
>> --- a/t/lib-gettext.sh
>> +++ b/t/lib-gettext.sh
>> @@ -14,12 +14,14 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
>> if test_have_prereq GETTEXT && ! test_have_prereq GETTEXT_POISON
>
> If we turn this line into:
>
> test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
>
> then people can see the error output of the setup step in verbose mode.
Ok, so it was not obviously "correct" after all ;-)
> +test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
> # is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
> - is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
> + is_IS_locale=$(locale -a | sed -n "/^is_IS\.[uU][tT][fF]-*8\$/{
Do we need to do this \$?
> p
> q
> - }')
> + }")
> # is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
> - is_IS_iso_locale=$(locale -a | sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
> + is_IS_iso_locale=$(locale -a | sed -n "/^is_IS\.[iI][sS][oO]8859-*1\$/{
> p
> q
> - }')
> + }")
>
> # Export them as an environment variable so the t0202/test.pl Perl
> # test can use it too
> @@ -37,7 +36,7 @@ then
> # Exporting for t0202/test.pl
> GETTEXT_LOCALE=1
> export GETTEXT_LOCALE
> - say "# lib-gettext: Found '$is_IS_locale' as an is_IS UTF-8 locale"
> + say "# lib-gettext: Found \"$is_IS_locale\" as an is_IS UTF-8 locale"
'\''?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] t0200: "locale" may not exist
2012-12-19 15:28 ` Junio C Hamano
@ 2012-12-19 15:32 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2012-12-19 15:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Wed, Dec 19, 2012 at 07:28:24AM -0800, Junio C Hamano wrote:
> > +test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
> > # is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
> > - is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
> > + is_IS_locale=$(locale -a | sed -n "/^is_IS\.[uU][tT][fF]-*8\$/{
>
> Do we need to do this \$?
I'm not sure. Sane shells leave "$/" untouched, but I do not know if we
need to be conservative.
> > - say "# lib-gettext: Found '$is_IS_locale' as an is_IS UTF-8 locale"
> > + say "# lib-gettext: Found \"$is_IS_locale\" as an is_IS UTF-8 locale"
>
> '\''?
Fine by me. I do not care either way in the output, and the escaped dq
is marginally more readable in the source. But either way the source is
pretty ugly. :)
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-12-19 15:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-19 6:47 [PATCH] t0200: "locale" may not exist Junio C Hamano
2012-12-19 13:18 ` Jeff King
2012-12-19 15:28 ` Junio C Hamano
2012-12-19 15:32 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).