unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] trunk r115886: Fix defun navigation in vc log view.
       [not found] <E1W04hL-0002hZ-NE@vcs.savannah.gnu.org>
@ 2014-01-06 15:05 ` Stefan Monnier
  2014-01-06 15:08   ` Daniel Colascione
  0 siblings, 1 reply; 2+ messages in thread
From: Stefan Monnier @ 2014-01-06 15:05 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: emacs-devel

> +	Fix defun navigation in vc log view.
[...]
>    (if (< arg 0)
> +      (log-view-end-of-defun (- arg))

This looks very fishy.  beginning-of-defun-function should never move to
the end of a defun, even when called with a negative argument.

So, if this is really correct, it needs a comment explaining why this is
the right thing to do.  And if you could add some regression tests for your
commit it would be even better.


        Stefan



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

* Re: [Emacs-diffs] trunk r115886: Fix defun navigation in vc log view.
  2014-01-06 15:05 ` [Emacs-diffs] trunk r115886: Fix defun navigation in vc log view Stefan Monnier
@ 2014-01-06 15:08   ` Daniel Colascione
  0 siblings, 0 replies; 2+ messages in thread
From: Daniel Colascione @ 2014-01-06 15:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

On 01/06/2014 07:05 AM, Stefan Monnier wrote:
>> +	Fix defun navigation in vc log view.
> [...]
>>     (if (< arg 0)
>> +      (log-view-end-of-defun (- arg))
>
> This looks very fishy.  beginning-of-defun-function should never move to
> the end of a defun, even when called with a negative argument.

In the vc log case, the end of the defun is the beginning of the next, 
so in this case, the code should be correct.

> So, if this is really correct, it needs a comment explaining why this is
> the right thing to do.  And if you could add some regression tests for your
> commit it would be even better.

Sure.



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

end of thread, other threads:[~2014-01-06 15:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <E1W04hL-0002hZ-NE@vcs.savannah.gnu.org>
2014-01-06 15:05 ` [Emacs-diffs] trunk r115886: Fix defun navigation in vc log view Stefan Monnier
2014-01-06 15:08   ` Daniel Colascione

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).