bug#2604: 23.0.91; key bindings in vc-annotate incompatible w/ Emacs 22

[Reiner Steib]

On Mon, Mar 09 2009, Dan Nicolaescu wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> writes:
>   > - D
>   > 
>   > Instead of showing diff of revision at line (as in Emacs 22), I get:
>   > 
>   > ,----[ *Messages* ]
>   > | vc-annotate-show-changeset-diff-revision-at-line:
>   > | The CVS backend does not support changeset diffs
>   > `----
>   > 
>   > Is this change intended?  
>
> It is. 
>
>   > I could understand that `D' should now work
>   > on change sets, but please at least mention it in NEWS and tell the
>   > user to try `d' instead or better just display the warning and than
>   > call `vc-annotate-show-diff-revision-at-line' if no change set
>   > operation is available.
>
> I am not sure that is better.  But if you can suggest a better wording
> for the error message, go for it.

How about this?  (Same in `log-view-diff-changeset', probably.)

--8<---------------cut here---------------start------------->8---
--- vc-annotate.el      09 Jan 2009 09:52:59 +0100      1.7
+++ vc-annotate.el      09 Mar 2009 20:35:28 +0100      
@@ -506,7 +506,10 @@
   "Visit the diff of the revision at line from its previous revision for all 
files in the changeset."
   (interactive)
   (when (eq 'file (vc-call-backend vc-annotate-backend 'revision-granularity))
-    (error "The %s backend does not support changeset diffs" 
vc-annotate-backend))
+    (error
+     (substitute-command-keys "The %s backend does not support changeset 
diffs.  \
+Use \\[vc-annotate-show-diff-revision-at-line] to diff this file.")
+     vc-annotate-backend))
   (vc-annotate-show-diff-revision-at-line-internal nil))
 
 (defun vc-annotate-warp-revision (revspec)
--8<---------------cut here---------------end--------------->8---

>   > - L
>   > 
>   > Instead of showing log of revision at line (as in Emacs 22), nothing
>   > happens.
>   > 
>   > If there's no useful binding for `L', why not bind it to
>   > `vc-annotate-show-log-revision-at-line' or at least tell the user to
>   > use `l' (lowercase L) and also document it in NEWS.
>
> That would be a bad idea.  

Do you also consider the following as bad?  If a future version
actually has a useful command for `L', this can be removed.

--8<---------------cut here---------------start------------->8---
--- vc-annotate.el.~1.7.~       2009-01-09 09:52:59.000000000 +0100
+++ vc-annotate.el      2009-03-09 20:45:47.000000000 +0100
@@ -124,6 +124,12 @@
     (define-key m "f" 'vc-annotate-find-revision-at-line)
     (define-key m "j" 'vc-annotate-revision-at-line)
     (define-key m "l" 'vc-annotate-show-log-revision-at-line)
+    (define-key m "L"
+      (lambda ()
+       (interactive)
+       (error
+        (substitute-command-keys
+         "Use \\[vc-annotate-show-log-revision-at-line] to view log."))))
     (define-key m "n" 'vc-annotate-next-revision)
     (define-key m "p" 'vc-annotate-prev-revision)
     (define-key m "w" 'vc-annotate-working-revision)
--8<---------------cut here---------------end--------------->8---

> All the bindings for vc-annotate were upper case.  And that was only
> because vc-annotate was derived from `view-mode' which was binding
> almost all lower case keys.  With more keys to bind we can add
> better functionality to vc-annotate (like the 'd' and 'D' bindings),
> so it would be a bad idea to 

I agree that the lower case binding are better, but the transition
should be made as smooth as possible.

> Yes, it would take a tiny bit of effort for the users to get used to
> the lower case bindings, but given that they are easier to type it
> should not be too bad.

My first reaction was that I really thought these feature won't work
anymore Emacs 23 because of the (partial) rewrite of Emacs' VC system.

> I'll make a note in NEWS about the changed bindings.

Thanks.

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/