unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] Fix tramp test
@ 2015-02-22 12:25 Przemysław Wojnowski
  2015-02-22 14:14 ` Michael Albinus
  0 siblings, 1 reply; 10+ messages in thread
From: Przemysław Wojnowski @ 2015-02-22 12:25 UTC (permalink / raw)
  To: emacs-devel

[-- Attachment #1: Type: text/plain, Size: 262 bytes --]

Hello everybody,

A tiny patch for a test that fails in non English systems. OS output summary is
language dependent and for example in Polish it displays "razem 1234" instead of
"total 1234".

Please let me know if it can't be applied for some reason.

Thanks!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-tramp-test.patch --]
[-- Type: text/x-patch; name="0001-Fix-tramp-test.patch", Size: 1407 bytes --]

From 214424ee32c7723e34924b220eb9f9a33881f9f0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Przemys=C5=82aw=20Wojnowski?= <esperanto@cumego.com>
Date: Sun, 22 Feb 2015 13:12:02 +0100
Subject: [PATCH] Fix tramp test.

Make summary line text matching working in non English systems too.
---
 test/ChangeLog                | 4 ++++
 test/automated/tramp-tests.el | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/test/ChangeLog b/test/ChangeLog
index 656124e..44e9444 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,5 +1,9 @@
 2015-02-22  Przemysław Wojnowski  <esperanto@cumego.com>
 
+	* automated/tramp-tests.el (tramp-test17-insert-directory):
+	Fix test. Make summary line text matching working in non English
+	systems too.
+
 	* automated/cl-lib-tests.el: New tests.
 	(cl-digit-char-p): Check returned value.
 
diff --git a/test/automated/tramp-tests.el b/test/automated/tramp-tests.el
index 2c4610c..e60a13f 100644
--- a/test/automated/tramp-tests.el
+++ b/test/automated/tramp-tests.el
@@ -955,7 +955,7 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
 	     (looking-at-p
 	      (concat
 	       ;; There might be a summary line.
-	       "\\(total.+[[:digit:]]+\n\\)?"
+	       "\\([[:alpha:]].+[[:digit:]]+\n\\)?"
 	       ;; We don't know in which order "." and ".." appear.
 	       "\\(.+ \\.?\\.\n\\)\\{2\\}"
 	       ".+ foo$")))))
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 12:25 [PATCH] Fix tramp test Przemysław Wojnowski
@ 2015-02-22 14:14 ` Michael Albinus
  2015-02-22 16:00   ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Albinus @ 2015-02-22 14:14 UTC (permalink / raw)
  To: Przemysław Wojnowski; +Cc: emacs-devel

Przemysław Wojnowski <esperanto@cumego.com> writes:

> Hello everybody,
>
> A tiny patch for a test that fails in non English systems. OS output summary is
> language dependent and for example in Polish it displays "razem 1234" instead of
> "total 1234".
>
> Please let me know if it can't be applied for some reason.
>
> diff --git a/test/automated/tramp-tests.el b/test/automated/tramp-tests.el
> index 2c4610c..e60a13f 100644
> --- a/test/automated/tramp-tests.el
> +++ b/test/automated/tramp-tests.el
> @@ -955,7 +955,7 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
>  	     (looking-at-p
>  	      (concat
>  	       ;; There might be a summary line.
> -	       "\\(total.+[[:digit:]]+\n\\)?"
> +	       "\\([[:alpha:]].+[[:digit:]]+\n\\)?"

Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"

>  	       ;; We don't know in which order "." and ".." appear.
>  	       "\\(.+ \\.?\\.\n\\)\\{2\\}"
>  	       ".+ foo$")))))

Beside of this, it looks OK to me. Please apply.

> Thanks!

Best regards, Michael.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 14:14 ` Michael Albinus
@ 2015-02-22 16:00   ` Eli Zaretskii
  2015-02-22 16:25     ` Michael Albinus
  2015-02-22 17:07     ` Eli Zaretskii
  0 siblings, 2 replies; 10+ messages in thread
From: Eli Zaretskii @ 2015-02-22 16:00 UTC (permalink / raw)
  To: Michael Albinus; +Cc: esperanto, emacs-devel

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Sun, 22 Feb 2015 15:14:57 +0100
> Cc: emacs-devel@gnu.org
> 
> > A tiny patch for a test that fails in non English systems. OS output summary is
> > language dependent and for example in Polish it displays "razem 1234" instead of
> > "total 1234".
> >
> > Please let me know if it can't be applied for some reason.
> >
> > diff --git a/test/automated/tramp-tests.el b/test/automated/tramp-tests.el
> > index 2c4610c..e60a13f 100644
> > --- a/test/automated/tramp-tests.el
> > +++ b/test/automated/tramp-tests.el
> > @@ -955,7 +955,7 @@ This tests also `file-directory-p' and `file-accessible-directory-p'."
> >  	     (looking-at-p
> >  	      (concat
> >  	       ;; There might be a summary line.
> > -	       "\\(total.+[[:digit:]]+\n\\)?"
> > +	       "\\([[:alpha:]].+[[:digit:]]+\n\\)?"
> 
> Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"

IMO, [:alpha:] is too indiscreet: it will match almost anything,
except in pure ASCII range.  Including some error message, for
example.

So I think it would be much better to keep the literal "total", and
instead set LC_MESSAGES=en_US.UTF-8, or something similar.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 16:00   ` Eli Zaretskii
@ 2015-02-22 16:25     ` Michael Albinus
  2015-02-22 17:10       ` Eli Zaretskii
  2015-02-22 17:07     ` Eli Zaretskii
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Albinus @ 2015-02-22 16:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: esperanto, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"
>
> IMO, [:alpha:] is too indiscreet: it will match almost anything,
> except in pure ASCII range.  Including some error message, for
> example.
>
> So I think it would be much better to keep the literal "total", and
> instead set LC_MESSAGES=en_US.UTF-8, or something similar.

Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
whatever is available on the remote system. But a user is free to
overwrite such settings.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 16:00   ` Eli Zaretskii
  2015-02-22 16:25     ` Michael Albinus
@ 2015-02-22 17:07     ` Eli Zaretskii
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2015-02-22 17:07 UTC (permalink / raw)
  To: michael.albinus, esperanto; +Cc: emacs-devel

> Date: Sun, 22 Feb 2015 18:00:44 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: esperanto@cumego.com, emacs-devel@gnu.org
> 
> IMO, [:alpha:] is too indiscreet

I mean "indiscriminate", sorry for an embarrassing typo.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 16:25     ` Michael Albinus
@ 2015-02-22 17:10       ` Eli Zaretskii
  2015-02-22 19:01         ` Michael Albinus
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2015-02-22 17:10 UTC (permalink / raw)
  To: Michael Albinus; +Cc: esperanto, emacs-devel

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: esperanto@cumego.com,  emacs-devel@gnu.org
> Date: Sun, 22 Feb 2015 17:25:14 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Better might be "\\([[:alpha:]]+[[:blank:]]+[[:digit:]]+\n\\)?"
> >
> > IMO, [:alpha:] is too indiscreet: it will match almost anything,
> > except in pure ASCII range.  Including some error message, for
> > example.
> >
> > So I think it would be much better to keep the literal "total", and
> > instead set LC_MESSAGES=en_US.UTF-8, or something similar.
> 
> Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
> whatever is available on the remote system. But a user is free to

Not sure what scenario are we talking about here.  Do you mean that
Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?

Or do you mean that you want to cater to users who have neither
en_US.utf8 nor C.utf8 installed?

In any case, I meant to set that as part of the test in question, not
in general.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 17:10       ` Eli Zaretskii
@ 2015-02-22 19:01         ` Michael Albinus
  2015-02-22 19:33           ` Przemysław Wojnowski
  2015-02-22 20:53           ` Andreas Schwab
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Albinus @ 2015-02-22 19:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: esperanto, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
>> whatever is available on the remote system. But a user is free to
>
> Not sure what scenario are we talking about here.  Do you mean that
> Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?

Yes.

> In any case, I meant to set that as part of the test in question, not
> in general.

Yep. I've committed a patch to the master which does this. It is not
perfect (there might be problems when tramp-test-temporary-file-directory
points to a real remote directory), but for the time being it is sufficient.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 19:01         ` Michael Albinus
@ 2015-02-22 19:33           ` Przemysław Wojnowski
  2015-02-22 20:53           ` Andreas Schwab
  1 sibling, 0 replies; 10+ messages in thread
From: Przemysław Wojnowski @ 2015-02-22 19:33 UTC (permalink / raw)
  To: Michael Albinus, Eli Zaretskii; +Cc: emacs-devel

It works. Thanks!



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 19:01         ` Michael Albinus
  2015-02-22 19:33           ` Przemysław Wojnowski
@ 2015-02-22 20:53           ` Andreas Schwab
  2015-02-22 21:46             ` Michael Albinus
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Schwab @ 2015-02-22 20:53 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Eli Zaretskii, esperanto, emacs-devel

Michael Albinus <michael.albinus@gmx.de> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Tramp sets LC_ALL to "en_US.utf8", "C.utf8", or "C". In this order,
>>> whatever is available on the remote system. But a user is free to
>>
>> Not sure what scenario are we talking about here.  Do you mean that
>> Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?
>
> Yes.

Note that LC_ALL has higher priority than LC_MESSAGES.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Fix tramp test
  2015-02-22 20:53           ` Andreas Schwab
@ 2015-02-22 21:46             ` Michael Albinus
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Albinus @ 2015-02-22 21:46 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Eli Zaretskii, esperanto, emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> Not sure what scenario are we talking about here.  Do you mean that
>>> Tramp sets LC_ALL, but the user overrides that with LC_MESSAGES?
>>
>> Yes.
>
> Note that LC_ALL has higher priority than LC_MESSAGES.

Don't know what a user does. Instead of setting LC_MESSAGES, she could
also set LC_ALL. That's why I fix it in the Tramp test; if a user
overwrites Tramp settings with something private, it's not Tramp's
problem(*).

> Andreas.

Best regards, Michael.

(*): Usually it is my problem then, handling bug reports ...



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-02-22 21:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-22 12:25 [PATCH] Fix tramp test Przemysław Wojnowski
2015-02-22 14:14 ` Michael Albinus
2015-02-22 16:00   ` Eli Zaretskii
2015-02-22 16:25     ` Michael Albinus
2015-02-22 17:10       ` Eli Zaretskii
2015-02-22 19:01         ` Michael Albinus
2015-02-22 19:33           ` Przemysław Wojnowski
2015-02-22 20:53           ` Andreas Schwab
2015-02-22 21:46             ` Michael Albinus
2015-02-22 17:07     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).