bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Eduard Wiebe]

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


 Hi all,

it bothers me that in some situation i cannot reach the head information
of the `vc-print-log' via `log-view-msg-prev'.

I fixed the issue by following simple patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: easy-mmode.el.patch --]
[-- Type: text/x-diff, Size: 530 bytes --]

=== modified file 'lisp/emacs-lisp/easy-mmode.el'
--- lisp/emacs-lisp/easy-mmode.el	2011-01-25 04:08:28 +0000
+++ lisp/emacs-lisp/easy-mmode.el	2011-08-27 16:50:49 +0000
@@ -578,7 +578,8 @@
 	 (if (< count 0) (,next-sym (- count))
            ,(funcall when-narrowed
              `(unless (re-search-backward ,re nil t count)
-                (error "No previous %s" ,name)))
+                (goto-char (point-min))
+		(error "No previous %s" ,name)))
            ,@body))
        (put ',prev-sym 'definition-name ',base))))
 

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Glenn Morris]

Eduard Wiebe wrote:

>               `(unless (re-search-backward ,re nil t count)
> -                (error "No previous %s" ,name)))
> +                (goto-char (point-min))
> +		(error "No previous %s" ,name)))

Or:

-             `(unless (re-search-backward ,re nil t count)
+             `(unless (re-search-backward ,re nil 'move count)

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Eduard Wiebe]

Glenn Morris <rgm@gnu.org> writes:

> Eduard Wiebe wrote:
>
>>               `(unless (re-search-backward ,re nil t count)
>> -                (error "No previous %s" ,name)))
>> +                (goto-char (point-min))
>> +		(error "No previous %s" ,name)))
>
> Or:
>
> -             `(unless (re-search-backward ,re nil t count)
> +             `(unless (re-search-backward ,re nil 'move count)

Yes, much nicer. Thanks.
Can you apply this patch to the trunk.

-- 
Eduard Wiebe

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Glenn Morris]

Eduard Wiebe wrote:

> Can you apply this patch to the trunk.

I'm not sure if the change is appropriate, for all users of that macro.
(I don't have much of an opinion about it.)

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Stefan Monnier]

>> Can you apply this patch to the trunk.
> I'm not sure if the change is appropriate, for all users of that macro.
> (I don't have much of an opinion about it.)

I think the change is a good one.  So if someone could check the various
uses of that macro to make sure it doesn't break anything, we can
install it.


        Stefan

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Eduard Wiebe]

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

>>> Can you apply this patch to the trunk.
>> I'm not sure if the change is appropriate, for all users of that macro.
>> (I don't have much of an opinion about it.)
>
> I think the change is a good one. 

1+

> So if someone could check the various uses of that macro to make sure
> it doesn't break anything, we can install it.

I found following uses of the macro:

  1889060   84 -rw-r--r--   1 ew       ew          80974 Sep  9 20:59 vc/diff-mode.el
  1892302   24 -rw-r--r--   1 ew       ew          21559 Aug 27 17:25 vc/log-view.el
  1892363   56 -rw-r--r--   1 ew       ew          51035 Aug 27 17:25 vc/smerge-mode.el
  1889741   20 -rw-r--r--   1 ew       ew          17799 Aug 27 17:25 vc/cvs-status.el

I patched `easy-mmode-define-navigation' as above and tested (marked as X) the
most functions without any trouble.

* diff-mode.el
  - [X] diff-hunk-prev
  - [X] diff-hunk-next
  - [X] diff-file-prev
  - [X] diff-file-next

* log-view.el
  - [X] log-view-msg-prev
  - [X] log-view-msg-next
  - [ ] log-view-file-prev
  - [ ] log-view-file-next

* smerge-mode
  - [ ] smerge-next
  - [ ] smerge-prev

* cvs-status
  - [X] cvs-status-next
  - [X] cvs-status-prev

-- 
Eduard Wiebe

bug#9409: 24.0.50; log-view-msg-prev, easy-mmode

[Stefan Monnier]

> I patched `easy-mmode-define-navigation' as above and tested (marked
> as X) the most functions without any trouble.

> * diff-mode.el
>   - [X] diff-hunk-prev
>   - [X] diff-hunk-next
>   - [X] diff-file-prev
>   - [X] diff-file-next

> * log-view.el
>   - [X] log-view-msg-prev
>   - [X] log-view-msg-next
>   - [ ] log-view-file-prev
>   - [ ] log-view-file-next

> * smerge-mode
>   - [ ] smerge-next
>   - [ ] smerge-prev

> * cvs-status
>   - [X] cvs-status-next
>   - [X] cvs-status-prev

If a problem shows up it's not going to be in the *-prev commands, but
in commands that call those functions internally.  So you need to check
the callers of diff-hunk-prev, diff-file-prev, etc..
It's always good to check with actual testing, but it's also important
to check by looking at the code.  IIUC this will be safe because in the
case where the behavior is changed, the function signals an error, so
only callers which catch this error (with condition-case) might
be affected.


        Stefan