Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Dmitry Gutov]

Hi Chong,

Chong Yidong <cyd@gnu.org> writes:
> ------------------------------------------------------------
> revno: 111061
> committer: Chong Yidong <cyd@gnu.org>
> branch nick: trunk
> timestamp: Sun 2012-12-02 14:06:32 +0800
> message:
>   Fix for bad test in ruby-mode-tests.el.
>   
>   * automated/ruby-mode-tests.el (ruby-add-log-current-method-examples):
>   Don't use loop macro, to allow automated testing to work.
> modified:
>   test/ChangeLog
>   test/automated/ruby-mode-tests.el

Is there some way to run all tests in automated fashion (which I broke)?

Would it have been better if I 'require'd cl-lib and used the prefixed
version of the macro?

Is it considered okay that you've replaced spaces with tabs?

> === modified file 'test/ChangeLog'
> --- a/test/ChangeLog	2012-11-20 04:24:09 +0000
> +++ b/test/ChangeLog	2012-12-02 06:06:32 +0000
> @@ -1,3 +1,9 @@
> +2012-12-02  Chong Yidong  <cyd@gnu.org>
> +
> +	* automated/ruby-mode-tests.el
> +	(ruby-add-log-current-method-examples): Don't use loop macro, to
> +	allow automated testing to work.
> +
>  2012-11-20  Stefan Monnier  <monnier@iro.umontreal.ca>
>  
>  	* automated/advice-tests.el (advice-tests--data): Remove.
>
> === modified file 'test/automated/ruby-mode-tests.el'
> --- a/test/automated/ruby-mode-tests.el	2012-11-14 12:17:21 +0000
> +++ b/test/automated/ruby-mode-tests.el	2012-12-02 06:06:32 +0000
> @@ -296,21 +296,23 @@
>    (let ((pairs '(("foo" . "#foo")
>                   ("C.foo" . ".foo")
>                   ("self.foo" . ".foo"))))
> -    (loop for (name . value) in pairs
> -          do (with-temp-buffer
> -               (insert (ruby-test-string
> -                        "module M
> +    (dolist (pair pairs)
> +      (let ((name  (car pair))
> +	    (value (cdr pair)))
> +	(with-temp-buffer
> +	  (insert (ruby-test-string
> +		   "module M
>                          |  class C
>                          |    def %s
>                          |    end
>                          |  end
>                          |end"
> -                        name))
> -               (ruby-mode)
> -               (search-backward "def")
> -               (forward-line)
> -               (should (string= (ruby-add-log-current-method)
> -                                (format "M::C%s" value)))))))
> +		   name))
> +	  (ruby-mode)
> +	  (search-backward "def")
> +	  (forward-line)
> +	  (should (string= (ruby-add-log-current-method)
> +			   (format "M::C%s" value))))))))
>  
>  (defvar ruby-block-test-example
>    (ruby-test-string

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Chong Yidong]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Is there some way to run all tests in automated fashion (which I
> broke)?

Do `make check' from the top level of the tree.

> Would it have been better if I 'require'd cl-lib and used the prefixed
> version of the macro?

Sure, that would probably have worked too.

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Dmitry Gutov]

On 03.12.2012 8:13, Chong Yidong wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>> Is there some way to run all tests in automated fashion (which I
>> broke)?
>
> Do `make check' from the top level of the tree.
>
>> Would it have been better if I 'require'd cl-lib and used the prefixed
>> version of the macro?
>
> Sure, that would probably have worked too.

Thanks for the answers.

What about the third question? Last time I've seen a discussion about 
whitespace style in Emacs codebase, I think it was somewhat of a 
consensus that spaces > tabs (which is also my preference), but doing 
whitespace-only commits was declared verboten, so I can't just revert 
that part of the change.

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stefan Monnier]

> What about the third question? Last time I've seen a discussion about
> whitespace style in Emacs codebase, I think it was somewhat of a consensus
> that spaces > tabs (which is also my preference), but doing whitespace-only
> commits was declared verboten, so I can't just revert that part of
> the change.

There are better things to do in life than worry about how the
whitespace you don't see is represented internally,


        Stefan

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Burton Samograd]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> There are better things to do in life than worry about how the
> whitespace you don't see is represented internally,

Not when someone has tabs set to 4 and someone else to 8, or 3, or 6 and
you're set to use spaces.

--
Burton Samograd

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Dmitry Gutov]

On 03.12.2012 23:48, Stefan Monnier wrote:
>> What about the third question? Last time I've seen a discussion about
>> whitespace style in Emacs codebase, I think it was somewhat of a consensus
>> that spaces > tabs (which is also my preference), but doing whitespace-only
>> commits was declared verboten, so I can't just revert that part of
>> the change.
>
> There are better things to do in life than worry about how the
> whitespace you don't see is represented internally,

Sure, there are. But I do see it, courtesy of whitespace-mode.

--Dmitry

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Dmitry Gutov]

Burton Samograd <burton@samograd.ca> writes:

> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>> There are better things to do in life than worry about how the
>> whitespace you don't see is represented internally,
>
> Not when someone has tabs set to 4 and someone else to 8, or 3, or 6 and
> you're set to use spaces.

This particular potential problem is taken care of in .dir-locals.el at
the top of the Emacs directory tree.

--Dmitry

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Burton Samograd]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Burton Samograd <burton@samograd.ca> writes:
>
>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>
>>> There are better things to do in life than worry about how the
>>> whitespace you don't see is represented internally,
>>
>> Not when someone has tabs set to 4 and someone else to 8, or 3, or 6 and
>> you're set to use spaces.
>
> This particular potential problem is taken care of in .dir-locals.el at
> the top of the Emacs directory tree.

If only we could use that solution at my work...

--
Burton Samograd

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stefan Monnier]

>> There are better things to do in life than worry about how the
>> whitespace you don't see is represented internally,
> Not when someone has tabs set to 4 and someone else to 8, or 3, or 6 and
> you're set to use spaces.

There are *much* better things to do in life than set tab-width to
a non-8 value in emacs-lisp-mode.


        Stefan

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stephen Leake]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> What about the third question? Last time I've seen a discussion about
>> whitespace style in Emacs codebase, I think it was somewhat of a consensus
>> that spaces > tabs (which is also my preference), but doing whitespace-only
>> commits was declared verboten, so I can't just revert that part of
>> the change.
>
> There are better things to do in life than worry about how the
> whitespace you don't see is represented internally,

Which is why there needs to be a clear policy, supported by Emacs tools,
that everyone knows and follows.

-- 
-- Stephe

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stefan Monnier]

> Which is why there needs to be a clear policy, supported by Emacs tools,
> that everyone knows and follows.

And the policy is: don't modify TABs or spaces on lines you do
not otherwise change.


        Stefan

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[martin rudalics]

 > And the policy is: don't modify TABs or spaces on lines you do
 > not otherwise change.

Easier said than done when you are used to invoke `indent-sexp' on the
sexp enclosing a small change.

martin

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stefan Monnier]

>> And the policy is: don't modify TABs or spaces on lines you do
>> not otherwise change.
> Easier said than done when you are used to invoke `indent-sexp' on the
> sexp enclosing a small change.

AFAIK indent-sexp won't touch a line that's already properly indented,
so TABs will only be changed to spaces (or vice versa) if you do
fix the indentation, which is OK.


        Stefan

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stephen Leake]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Which is why there needs to be a clear policy, supported by Emacs tools,
>> that everyone knows and follows.
>
> And the policy is: don't modify TABs or spaces on lines you do
> not otherwise change.

That's clear.

But the other part of the question was: are we supposed to use tabs or
spaces when editing new or old code?

It's easy enough to tell Emacs which to use, and it's best if all code
in a project follows a common standard.

I don't see any documentation on this in the various READMEs in the
Emacs source tree (but I may have missed something).

I thought this was mentioned in the Gnu coding standards (I have a note
in my .emacs for C style that says the Gnu coding standard requires tabs),
but I don't see it in the current version at
http://www.gnu.org/prep/standards/standards.html

-- 
-- Stephe

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stefan Monnier]

> But the other part of the question was: are we supposed to use tabs or
> spaces when editing new or old code?

Irrelevant.  Use whichever you like.

> it's best if all code in a project follows a common standard.

Yes, but that doesn't mean you need to pay attention to
irrelevant details.


        Stefan

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[martin rudalics]

 > AFAIK indent-sexp won't touch a line that's already properly indented,
 > so TABs will only be changed to spaces (or vice versa) if you do
 > fix the indentation, which is OK.

Right.  But if you, say, introduce a new let binding somewhere at the
beginning of a defun and later on decide that you don't need it but do
want to keep other changes you made in between, reindenting will leave
whitespace changes in the new code.

martin

Re: /srv/bzr/emacs/trunk r111061: Fix for bad test in ruby-mode-tests.el.

[Stefan Monnier]

>> AFAIK indent-sexp won't touch a line that's already properly indented,
>> so TABs will only be changed to spaces (or vice versa) if you do
>> fix the indentation, which is OK.
> Right.  But if you, say, introduce a new let binding somewhere at the
> beginning of a defun and later on decide that you don't need it but do
> want to keep other changes you made in between, reindenting will leave
> whitespace changes in the new code.

Better not worry about it.


        Stefan